Changeset 247222 in webkit


Ignore:
Timestamp:
Jul 8, 2019 11:59:49 AM (5 years ago)
Author:
Wenson Hsieh
Message:

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.

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.

  1. Run a promise that resolves immediately; the promise callback restores focus to the originally focused

element prior to (1).

  1. Invoke programmatic paste using document.execCommand("Paste").
  2. 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.

  1. 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.

  1. 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)).

  1. 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.
Location:
trunk
Files:
2 added
5 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r247213 r247222  
     12019-07-08  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        Unable to paste from Notes into Excel 365 spreadsheet
     4        https://bugs.webkit.org/show_bug.cgi?id=199565
     5        <rdar://problem/43615497>
     6
     7        Reviewed by Chris Dumez.
     8
     9        Add a test to verify that promises scheduled right before a programmatic paste resolve in the middle of the
     10        paste, while creating a document for web content sanitization. See WebCore ChangeLog for more details.
     11
     12        * editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content-expected.txt: Added.
     13        * editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content.html: Added.
     14
    1152019-07-08  Chris Dumez  <cdumez@apple.com>
    216

  • trunk/Source/WebCore/ChangeLog

    r247220 r247222  
     12019-07-08  Wenson Hsieh  <wenson_hsieh@apple.com>
     2
     3        Unable to paste from Notes into Excel 365 spreadsheet
     4        https://bugs.webkit.org/show_bug.cgi?id=199565
     5        <rdar://problem/43615497>
     6
     7        Reviewed by Chris Dumez.
     8
     9        When pasting into Microsoft Excel 365, the copied data is all inserted into a single cell, even when copying a
     10        table. To understand why this happens, we first need to understand how Excel's logic for handling paste works.
     11        When tapping on the "Paste" button, Excel performs and expects the following:
     12
     13        1.  Before triggering programmatic paste, move focus into a hidden contenteditable area specifically intended to
     14            capture pasted content.
     15        2.  Run a promise that resolves immediately; the promise callback restores focus to the originally focused
     16            element prior to (1).
     17        3.  Invoke programmatic paste using `document.execCommand("Paste")`.
     18        4.  The callback scheduled in step (2) then runs, restoring focus to the main editable element representing a
     19            table cell.
     20
     21        However, what ends up happening is this:
     22
     23        Steps (1)-(3): same as before.
     24        4.  We (WebKit) create a temporary Page for the purposes of sanitizing copied web content before exposing it to
     25            the paste handler. This involves creating and loading a document; when this is finished, we call into
     26            Document::finishedParsing which flushes the microtask queue.
     27        5.  This causes us to immediately run the microtask enqueued in step (2), which restores focus to the previously
     28            focused element (importantly, this is not the element that was focused in step (1)).
     29        6.  The paste commences, and inserts the sanitized fragment into the originally focused element rather than the
     30            content editable area intended to capture pasted content.
     31
     32        Excel's script then gets confused, and does not end up using their special paste logic to handle the paste. The
     33        pasted content is instead just inserted as plain text in a cell. To address this, we simply prevent document
     34        load in the Page for web content sanitization from triggering a microtask checkpoint; this allows any scheduled
     35        main thread microtasks to be deferred until the next turn of the runloop.
     36
     37        Test: editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content.html
     38
     39        * dom/Document.cpp:
     40        (WebCore::Document::finishedParsing):
     41
     42        Don't immediately dispatch microtasks when we finish document parsing, in the case where the page is intended
     43        only for web content sanitization, since this may end up executing script in the original document. As explained
     44        above, this causes compatibility issues when pasting in Excel.
     45
     46        * editing/markup.cpp:
     47        (WebCore::createPageForSanitizingWebContent):
     48
     49        When creating a page for sanitizing web content, mark it as such.
     50
     51        * page/Page.h:
     52
     53        Add a new flag to indicate that a Page is only intended for sanitizing web content.
     54
     55        (WebCore::Page::setIsForSanitizingWebContent):
     56        (WebCore::Page::isForSanitizingWebContent const):
     57
    1582019-07-08  Konstantin Tokarev  <annulen@yandex.ru>
    259

  • trunk/Source/WebCore/dom/Document.cpp

    r247184 r247222  
    56715671        m_documentTiming.domContentLoadedEventStart = MonotonicTime::now();
    56725672
    5673     // FIXME: Schdule a task to fire DOMContentLoaded event instead. See webkit.org/b/82931
    5674     MicrotaskQueue::mainThreadQueue().performMicrotaskCheckpoint();
     5673    if (!page() || !page()->isForSanitizingWebContent()) {
     5674        // FIXME: Schedule a task to fire DOMContentLoaded event instead. See webkit.org/b/82931
     5675        MicrotaskQueue::mainThreadQueue().performMicrotaskCheckpoint();
     5676    }
    56755677
    56765678    dispatchEvent(Event::create(eventNames().DOMContentLoadedEvent, Event::CanBubble::Yes, Event::IsCancelable::No));

  • trunk/Source/WebCore/editing/markup.cpp

    r246596 r247222  
    180180   
    181181    auto page = std::make_unique<Page>(WTFMove(pageConfiguration));
     182    page->setIsForSanitizingWebContent();
    182183    page->settings().setMediaEnabled(false);
    183184    page->settings().setScriptEnabled(false);

  • trunk/Source/WebCore/page/Page.h

    r246938 r247222  
    654654    bool expectsWheelEventTriggers() const { return !!m_testTrigger; }
    655655
     656    void setIsForSanitizingWebContent() { m_isForSanitizingWebContent = true; }
     657    bool isForSanitizingWebContent() const { return m_isForSanitizingWebContent; }
     658
    656659#if ENABLE(VIDEO)
    657660    bool allowsMediaDocumentInlinePlayback() const { return m_allowsMediaDocumentInlinePlayback; }
    … …  
    992995    bool m_mediaBufferingIsSuspended { false };
    993996    bool m_inUpdateRendering { false };
     997
     998    bool m_isForSanitizingWebContent { false };
    994999};
    9951000
Note: See TracChangeset for help on using the changeset viewer.