Unable to paste from Notes into Excel 365 spreadsheet
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Jul 2019 18:59:49 +0000 (18:59 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 8 Jul 2019 18:59:49 +0000 (18:59 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199565
<rdar://problem/43615497>

Reviewed by Chris Dumez.

Source/WebCore:

When pasting into Microsoft Excel 365, the copied data is all inserted into a single cell, even when copying a
table. To understand why this happens, we first need to understand how Excel's logic for handling paste works.
When tapping on the "Paste" button, Excel performs and expects the following:

1.  Before triggering programmatic paste, move focus into a hidden contenteditable area specifically intended to
    capture pasted content.
2.  Run a promise that resolves immediately; the promise callback restores focus to the originally focused
    element prior to (1).
3.  Invoke programmatic paste using `document.execCommand("Paste")`.
4.  The callback scheduled in step (2) then runs, restoring focus to the main editable element representing a
    table cell.

However, what ends up happening is this:

Steps (1)-(3): same as before.
4.  We (WebKit) create a temporary Page for the purposes of sanitizing copied web content before exposing it to
    the paste handler. This involves creating and loading a document; when this is finished, we call into
    Document::finishedParsing which flushes the microtask queue.
5.  This causes us to immediately run the microtask enqueued in step (2), which restores focus to the previously
    focused element (importantly, this is not the element that was focused in step (1)).
6.  The paste commences, and inserts the sanitized fragment into the originally focused element rather than the
    content editable area intended to capture pasted content.

Excel's script then gets confused, and does not end up using their special paste logic to handle the paste. The
pasted content is instead just inserted as plain text in a cell. To address this, we simply prevent document
load in the Page for web content sanitization from triggering a microtask checkpoint; this allows any scheduled
main thread microtasks to be deferred until the next turn of the runloop.

Test: editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content.html

* dom/Document.cpp:
(WebCore::Document::finishedParsing):

Don't immediately dispatch microtasks when we finish document parsing, in the case where the page is intended
only for web content sanitization, since this may end up executing script in the original document. As explained
above, this causes compatibility issues when pasting in Excel.

* editing/markup.cpp:
(WebCore::createPageForSanitizingWebContent):

When creating a page for sanitizing web content, mark it as such.

* page/Page.h:

Add a new flag to indicate that a Page is only intended for sanitizing web content.

(WebCore::Page::setIsForSanitizingWebContent):
(WebCore::Page::isForSanitizingWebContent const):

LayoutTests:

Add a test to verify that promises scheduled right before a programmatic paste resolve in the middle of the
paste, while creating a document for web content sanitization. See WebCore ChangeLog for more details.

* editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content-expected.txt: Added.
* editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@247222 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content-expected.txt [new file with mode: 0644]
LayoutTests/editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/Document.cpp
Source/WebCore/editing/markup.cpp
Source/WebCore/page/Page.h

index 2cec6d4..1b4e25f 100644 (file)
@@ -1,3 +1,17 @@
+2019-07-08  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Unable to paste from Notes into Excel 365 spreadsheet
+        https://bugs.webkit.org/show_bug.cgi?id=199565
+        <rdar://problem/43615497>
+
+        Reviewed by Chris Dumez.
+
+        Add a test to verify that promises scheduled right before a programmatic paste resolve in the middle of the
+        paste, while creating a document for web content sanitization. See WebCore ChangeLog for more details.
+
+        * editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content-expected.txt: Added.
+        * editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content.html: Added.
+
 2019-07-08  Chris Dumez  <cdumez@apple.com>
 
         Unable to play videos on xfinity.com/stream on macOS Catalina
diff --git a/LayoutTests/editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content-expected.txt b/LayoutTests/editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content-expected.txt
new file mode 100644 (file)
index 0000000..d1801a7
--- /dev/null
@@ -0,0 +1,13 @@
+Click here to copy
+Verifies that a promise scheduled right before a programmatically triggered paste does not resolve during the paste. To test manually, tap the frame to copy some text, and then tap the editable area to paste. The promise should run after the paste event handler.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+PASS Focused editor.
+PASS Handled paste.
+PASS Focused textarea.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content.html b/LayoutTests/editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content.html
new file mode 100644 (file)
index 0000000..08f8272
--- /dev/null
@@ -0,0 +1,83 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+<html>
+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+<head>
+<script src="../../resources/js-test.js"></script>
+<script src="../../resources/ui-helper.js"></script>
+<style>
+body {
+    margin: 0;
+}
+
+#copy {
+    width: 100%;
+    height: 50px;
+    border: 1px dashed black;
+}
+
+#editor {
+    width: 100%;
+    height: 100px;
+    border: 1px dashed tomato;
+    text-align: center;
+}
+</style>
+</head>
+<body>
+<div id="editor" contenteditable></div>
+<iframe id="copy" src="data:text/html,<div id='copy' style='font-size: 40px; text-align: center;'>Click here to copy</div>
+    <script>
+    copy.addEventListener('mousedown', event => {
+        getSelection().selectAllChildren(copy);
+        document.execCommand('Copy');
+        getSelection().removeAllRanges();
+        event.preventDefault();
+    });
+    </script>"></iframe>
+<textarea></textarea>
+<div id="description"></div>
+<div id="console"></div>
+<script>
+jsTestIsAsync = true;
+
+description("Verifies that a promise scheduled right before a programmatically triggered paste does not resolve during the paste. To test manually, tap the frame to copy some text, and then tap the editable area to paste. The promise should run after the paste event handler.");
+
+textarea = document.querySelector("textarea");
+frame = document.querySelector("iframe");
+editor = document.getElementById("editor");
+progress = 0;
+
+function checkDone() {
+    if (++progress == 3)
+        finishJSTest();
+}
+
+addEventListener("load", async () => {
+    if (!window.testRunner)
+        return;
+
+    await UIHelper.activateElement(frame);
+    await UIHelper.activateElement(editor);
+    checkDone();
+});
+
+editor.addEventListener("mousedown", event => {
+    editor.focus();
+    Promise.resolve().then(() => textarea.focus());
+    document.execCommand("Paste");
+    event.preventDefault();
+});
+
+editor.addEventListener("paste", () => {
+    testPassed("Handled paste.");
+    checkDone();
+});
+
+editor.addEventListener("focus", () => testPassed("Focused editor."));
+textarea.addEventListener("focus", () => {
+    testPassed("Focused textarea.");
+    checkDone();
+});
+</script>
+</body>
+</html>
index 1c1e6ac..4e1e249 100644 (file)
@@ -1,3 +1,60 @@
+2019-07-08  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Unable to paste from Notes into Excel 365 spreadsheet
+        https://bugs.webkit.org/show_bug.cgi?id=199565
+        <rdar://problem/43615497>
+
+        Reviewed by Chris Dumez.
+
+        When pasting into Microsoft Excel 365, the copied data is all inserted into a single cell, even when copying a
+        table. To understand why this happens, we first need to understand how Excel's logic for handling paste works.
+        When tapping on the "Paste" button, Excel performs and expects the following:
+
+        1.  Before triggering programmatic paste, move focus into a hidden contenteditable area specifically intended to
+            capture pasted content.
+        2.  Run a promise that resolves immediately; the promise callback restores focus to the originally focused
+            element prior to (1).
+        3.  Invoke programmatic paste using `document.execCommand("Paste")`.
+        4.  The callback scheduled in step (2) then runs, restoring focus to the main editable element representing a
+            table cell.
+
+        However, what ends up happening is this:
+
+        Steps (1)-(3): same as before.
+        4.  We (WebKit) create a temporary Page for the purposes of sanitizing copied web content before exposing it to
+            the paste handler. This involves creating and loading a document; when this is finished, we call into
+            Document::finishedParsing which flushes the microtask queue.
+        5.  This causes us to immediately run the microtask enqueued in step (2), which restores focus to the previously
+            focused element (importantly, this is not the element that was focused in step (1)).
+        6.  The paste commences, and inserts the sanitized fragment into the originally focused element rather than the
+            content editable area intended to capture pasted content.
+
+        Excel's script then gets confused, and does not end up using their special paste logic to handle the paste. The
+        pasted content is instead just inserted as plain text in a cell. To address this, we simply prevent document
+        load in the Page for web content sanitization from triggering a microtask checkpoint; this allows any scheduled
+        main thread microtasks to be deferred until the next turn of the runloop.
+
+        Test: editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content.html
+
+        * dom/Document.cpp:
+        (WebCore::Document::finishedParsing):
+
+        Don't immediately dispatch microtasks when we finish document parsing, in the case where the page is intended
+        only for web content sanitization, since this may end up executing script in the original document. As explained
+        above, this causes compatibility issues when pasting in Excel.
+
+        * editing/markup.cpp:
+        (WebCore::createPageForSanitizingWebContent):
+
+        When creating a page for sanitizing web content, mark it as such.
+
+        * page/Page.h:
+
+        Add a new flag to indicate that a Page is only intended for sanitizing web content.
+
+        (WebCore::Page::setIsForSanitizingWebContent):
+        (WebCore::Page::isForSanitizingWebContent const):
+
 2019-07-08  Konstantin Tokarev  <annulen@yandex.ru>
 
         Remove unused #include "ImageBufferData.h"
index 5cfdaaf..57ab3e1 100644 (file)
@@ -5670,8 +5670,10 @@ void Document::finishedParsing()
     if (!m_documentTiming.domContentLoadedEventStart)
         m_documentTiming.domContentLoadedEventStart = MonotonicTime::now();
 
-    // FIXME: Schdule a task to fire DOMContentLoaded event instead. See webkit.org/b/82931
-    MicrotaskQueue::mainThreadQueue().performMicrotaskCheckpoint();
+    if (!page() || !page()->isForSanitizingWebContent()) {
+        // FIXME: Schedule a task to fire DOMContentLoaded event instead. See webkit.org/b/82931
+        MicrotaskQueue::mainThreadQueue().performMicrotaskCheckpoint();
+    }
 
     dispatchEvent(Event::create(eventNames().DOMContentLoadedEvent, Event::CanBubble::Yes, Event::IsCancelable::No));
 
index 0f7cea7..203cc47 100644 (file)
@@ -179,6 +179,7 @@ std::unique_ptr<Page> createPageForSanitizingWebContent()
     auto pageConfiguration = pageConfigurationWithEmptyClients();
     
     auto page = std::make_unique<Page>(WTFMove(pageConfiguration));
+    page->setIsForSanitizingWebContent();
     page->settings().setMediaEnabled(false);
     page->settings().setScriptEnabled(false);
     page->settings().setPluginsEnabled(false);
index 2fab6e2..ef0e830 100644 (file)
@@ -653,6 +653,9 @@ public:
     void clearTrigger() { m_testTrigger = nullptr; }
     bool expectsWheelEventTriggers() const { return !!m_testTrigger; }
 
+    void setIsForSanitizingWebContent() { m_isForSanitizingWebContent = true; }
+    bool isForSanitizingWebContent() const { return m_isForSanitizingWebContent; }
+
 #if ENABLE(VIDEO)
     bool allowsMediaDocumentInlinePlayback() const { return m_allowsMediaDocumentInlinePlayback; }
     WEBCORE_EXPORT void setAllowsMediaDocumentInlinePlayback(bool);
@@ -991,6 +994,8 @@ private:
     bool m_mediaPlaybackIsSuspended { false };
     bool m_mediaBufferingIsSuspended { false };
     bool m_inUpdateRendering { false };
+
+    bool m_isForSanitizingWebContent { false };
 };
 
 inline PageGroup& Page::group()