- Revision
- 247222
- Author
- wenson_hs...@apple.com
- Date
- 2019-07-08 11:59:49 -0700 (Mon, 08 Jul 2019)
Log 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.
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (247221 => 247222)
--- trunk/LayoutTests/ChangeLog 2019-07-08 18:55:22 UTC (rev 247221)
+++ trunk/LayoutTests/ChangeLog 2019-07-08 18:59:49 UTC (rev 247222)
@@ -1,3 +1,17 @@
+2019-07-08 Wenson Hsieh <wenson_hs...@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 <cdu...@apple.com>
Unable to play videos on xfinity.com/stream on macOS Catalina
Added: trunk/LayoutTests/editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content-expected.txt (0 => 247222)
--- trunk/LayoutTests/editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content-expected.txt (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content-expected.txt 2019-07-08 18:59:49 UTC (rev 247222)
@@ -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
+
Added: trunk/LayoutTests/editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content.html (0 => 247222)
--- trunk/LayoutTests/editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content.html (rev 0)
+++ trunk/LayoutTests/editing/pasteboard/paste-does-not-fire-promises-while-sanitizing-web-content.html 2019-07-08 18:59:49 UTC (rev 247222)
@@ -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=""
+<script src=""
+<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="" 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>
Modified: trunk/Source/WebCore/ChangeLog (247221 => 247222)
--- trunk/Source/WebCore/ChangeLog 2019-07-08 18:55:22 UTC (rev 247221)
+++ trunk/Source/WebCore/ChangeLog 2019-07-08 18:59:49 UTC (rev 247222)
@@ -1,3 +1,60 @@
+2019-07-08 Wenson Hsieh <wenson_hs...@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 <annu...@yandex.ru>
Remove unused #include "ImageBufferData.h"
Modified: trunk/Source/WebCore/dom/Document.cpp (247221 => 247222)
--- trunk/Source/WebCore/dom/Document.cpp 2019-07-08 18:55:22 UTC (rev 247221)
+++ trunk/Source/WebCore/dom/Document.cpp 2019-07-08 18:59:49 UTC (rev 247222)
@@ -5670,8 +5670,10 @@
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));
Modified: trunk/Source/WebCore/editing/markup.cpp (247221 => 247222)
--- trunk/Source/WebCore/editing/markup.cpp 2019-07-08 18:55:22 UTC (rev 247221)
+++ trunk/Source/WebCore/editing/markup.cpp 2019-07-08 18:59:49 UTC (rev 247222)
@@ -179,6 +179,7 @@
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);
Modified: trunk/Source/WebCore/page/Page.h (247221 => 247222)
--- trunk/Source/WebCore/page/Page.h 2019-07-08 18:55:22 UTC (rev 247221)
+++ trunk/Source/WebCore/page/Page.h 2019-07-08 18:59:49 UTC (rev 247222)
@@ -653,6 +653,9 @@
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 @@
bool m_mediaPlaybackIsSuspended { false };
bool m_mediaBufferingIsSuspended { false };
bool m_inUpdateRendering { false };
+
+ bool m_isForSanitizingWebContent { false };
};
inline PageGroup& Page::group()
