Prioritize pasting files over pasting HTML content (#1148)

* Prioritize pasting files over pasting HTML content When right clicking and pasting an image the dataTransfer object contains a file and text/html. If we paste the HTML content, the result is often a broken image. This change prioritizes pasting the file over pasting the HTML content. The gotcha is that when pasting text from MS Word, the dataTransfer object contains a File with a screenshot of the text and text/html with the actual text. We need to check if the paste is from MS Word and if so, we should paste the text instead of the screenshot. * Prevent duplicate file pastes This change prevents duplicate file pastes by checking if the paste event is a file paste before handling it. This is necessary because Safari doesn't support `beforeinput.insertFromPaste` for files, so we are handling file pastes in the paste event handler too. We need to check in the `beforeinput.insertFromPaste` event handler if the event will be also handled by the paste event handler, to avoid duplicate file pastes.
basecamp · Jul 15, 2024 · 831697b · 831697b
1 parent db9c35d
commit 831697b
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 14 deletions.
diff --git a/src/test/system/level_2_input_test.js b/src/test/system/level_2_input_test.js
@@ -231,7 +231,14 @@ testGroup("Level 2 Input", testOptions, () => {
   test("pasting text from MS Word", async () => {
     const file = await createFile()
     const dataTransfer = createDataTransfer({
-      "text/html": "<span class=\"MsoNormal\">abc</span>",
+      "text/html": `<html xmlns:o="urn:schemas-microsoft-com:office:office"
+        xmlns:w="urn:schemas-microsoft-com:office:word"
+        xmlns:m="http://schemas.microsoft.com/office/2004/12/omml"
+        xmlns="http://www.w3.org/TR/REC-html40">
+        <body>
+          <span class="MsoNormal">abc</span>
+        </body>
+      </html>`,
       "text/plain": "abc",
       Files: [ file ],
     })

diff --git a/src/trix/controllers/level_2_input_controller.js b/src/trix/controllers/level_2_input_controller.js
@@ -2,7 +2,7 @@ import { getAllAttributeNames, squishBreakableWhitespace } from "trix/core/helpe
 import InputController from "trix/controllers/input_controller"
 import * as config from "trix/config"
 
-import { dataTransferIsPlainText, keyEventIsKeyboardCommand, objectsAreEqual } from "trix/core/helpers"
+import { dataTransferIsMsOfficePaste, dataTransferIsPlainText, keyEventIsKeyboardCommand, objectsAreEqual } from "trix/core/helpers"
 
 import { selectionChangeObserver } from "trix/observers/selection_change_observer"
 
@@ -360,6 +360,7 @@ export default class Level2InputController extends InputController {
     insertFromPaste() {
       const { dataTransfer } = this.event
       const paste = { dataTransfer }
+
       const href = dataTransfer.getData("URL")
       const html = dataTransfer.getData("text/html")
 
@@ -378,7 +379,6 @@ export default class Level2InputController extends InputController {
         this.withTargetDOMRange(function() {
           return this.responder?.insertHTML(paste.html)
         })
-
         this.afterRender = () => {
           return this.delegate?.inputControllerDidPaste(paste)
         }
@@ -393,26 +393,25 @@ export default class Level2InputController extends InputController {
         this.afterRender = () => {
           return this.delegate?.inputControllerDidPaste(paste)
         }
-      } else if (html) {
-        this.event.preventDefault()
-        paste.type = "text/html"
-        paste.html = html
+      } else if (processableFilePaste(this.event)) {
+        paste.type = "File"
+        paste.file = dataTransfer.files[0]
         this.delegate?.inputControllerWillPaste(paste)
         this.withTargetDOMRange(function() {
-          return this.responder?.insertHTML(paste.html)
+          return this.responder?.insertFile(paste.file)
         })
 
         this.afterRender = () => {
           return this.delegate?.inputControllerDidPaste(paste)
         }
-      } else if (dataTransfer.files?.length) {
-        paste.type = "File"
-        paste.file = dataTransfer.files[0]
+      } else if (html) {
+        this.event.preventDefault()
+        paste.type = "text/html"
+        paste.html = html
         this.delegate?.inputControllerWillPaste(paste)
         this.withTargetDOMRange(function() {
-          return this.responder?.insertFile(paste.file)
+          return this.responder?.insertHTML(paste.html)
         })
-
         this.afterRender = () => {
           return this.delegate?.inputControllerDidPaste(paste)
         }
@@ -582,10 +581,20 @@ const staticRangeToRange = function(staticRange) {
 
 const dragEventHasFiles = (event) => Array.from(event.dataTransfer?.types || []).includes("Files")
 
+const processableFilePaste = (event) => {
+  // Paste events that only have files are handled by the paste event handler,
+  // to work around Safari not supporting beforeinput.insertFromPaste for files.
+
+  // MS Office text pastes include a file with a screenshot of the text, but we should
+  // handle them as text pastes.
+  return event.dataTransfer.files?.[0] && !pasteEventHasFilesOnly(event) && !dataTransferIsMsOfficePaste(event)
+}
+
 const pasteEventHasFilesOnly = function(event) {
   const clipboard = event.clipboardData
   if (clipboard) {
-    return clipboard.types.includes("Files") && clipboard.types.length === 1 && clipboard.files.length >= 1
+    const fileTypes = Array.from(clipboard.types).filter((type) => type.match(/file/i)) // "Files", "application/x-moz-file"
+    return fileTypes.length === clipboard.types.length && clipboard.files.length >= 1
   }
 }
 

diff --git a/src/trix/core/helpers/events.js b/src/trix/core/helpers/events.js
@@ -14,6 +14,12 @@ export const dataTransferIsPlainText = function(dataTransfer) {
   }
 }
 
+export const dataTransferIsMsOfficePaste = ({ dataTransfer }) => {
+  return dataTransfer.types.includes("Files") &&
+    dataTransfer.types.includes("text/html") &&
+    dataTransfer.getData("text/html").includes("urn:schemas-microsoft-com:office:office")
+}
+
 export const dataTransferIsWritable = function(dataTransfer) {
   if (!dataTransfer?.setData) return false