Attachment 373609 Details for Bug 199565 – Patch

[patch] Patch

bug-199565-20190707173619.patch (text/plain), 7.11 KB, created by Wenson Hsieh on 2019-07-07 17:36:20 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Wenson Hsieh

Created: 2019-07-07 17:36:20 PDT

Size: 7.11 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

Actions: View | Formatted Diff | Diff

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