Attachment 373610 Details for Bug 199565 – Actually add the test

[patch] Actually add the test

bug-199565-20190707175013.patch (text/plain), 10.73 KB, created by Wenson Hsieh on 2019-07-07 17:50:14 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-07-07 17:50:14 PDT

Size: 10.73 KB

patch

obsolete

>Subversion Revision: 247162
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index d87dbc82fe93bbba7d2934af0a4a86abda92cb82..9621d92f716d71e4a342d06dffadb4b702ad0e11 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,59 @@
>+2019-07-07  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 NOBODY (OOPS!).
>+
>+        When 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) creates 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.
>+
>+        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-05  Ryan Haddad  <ryanhaddad@apple.com>
> 
>         Unreviewed, rolling out r247123.
>diff --git a/Source/WebCore/dom/Document.cpp b/Source/WebCore/dom/Document.cpp
>index 78e59127508d98f97539858761b4dd3649e2aad2..bd12169fcde519f009c7fea00fe5c07b125a2839 100644
>--- a/Source/WebCore/dom/Document.cpp
>+++ b/Source/WebCore/dom/Document.cpp
>@@ -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));
> 
>diff --git a/Source/WebCore/editing/markup.cpp b/Source/WebCore/editing/markup.cpp
>index 0f7cea703e5fcb8c26e15ac03d041e0140ad6c91..03f035db7267d64ae30c84d926aad914943d31ba 100644
>--- a/Source/WebCore/editing/markup.cpp
>+++ b/Source/WebCore/editing/markup.cpp
>@@ -179,6 +179,7 @@ std::unique_ptr<Page> createPageForSanitizingWebContent()
>     auto pageConfiguration = pageConfigurationWithEmptyClients();
>     
>     auto page = std::make_unique<Page>(WTFMove(pageConfiguration));
>+    page->setIsForSanitizingWebContent(true);
>     page->settings().setMediaEnabled(false);
>     page->settings().setScriptEnabled(false);
>     page->settings().setPluginsEnabled(false);
>diff --git a/Source/WebCore/page/Page.h b/Source/WebCore/page/Page.h
>index 2fab6e2e523c9553fb9bf84cbe0c4c9c41620593..b1b1cfcc4cf5d0b1fbe7c7ae8f5b67ca9d188ccf 100644
>--- a/Source/WebCore/page/Page.h
>+++ b/Source/WebCore/page/Page.h
>@@ -653,6 +653,9 @@ public:
>     void clearTrigger() { m_testTrigger = nullptr; }
>     bool expectsWheelEventTriggers() const { return !!m_testTrigger; }
> 
>+    void setIsForSanitizingWebContent(bool isForSanitizingWebContent) { m_isForSanitizingWebContent = isForSanitizingWebContent; }
>+    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()
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 53fe5808e8e3842f997eb74a06d51c7d70fa0b83..998e41ba1ecd27d2b1ec144925d5b7ce4d4e9be6 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,17 @@
>+2019-07-07  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 NOBODY (OOPS!).
>+
>+        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-05  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         Click events on outer page are not being dispatched correctly after touch-zooming within an iframe
>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
>index 0000000000000000000000000000000000000000..d1801a7f478d0a68e7f57ad21543a7e30bbd4519
>--- /dev/null
>+++ b/LayoutTests/editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content-expected.txt
>@@ -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
>index 0000000000000000000000000000000000000000..08f8272bc02736c2b2116fde6d59bc9777c51f1d
>--- /dev/null
>+++ b/LayoutTests/editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content.html
>@@ -0,0 +1,83 @@
>+<!DOCTYPE html> 
>+<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>

Actions: View | Formatted Diff | Diff

Attachments on bug 199565: 373609 | 373610 | 373624 | 373628