Allow the async clipboard API to write data when copying via menu action or key binding
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Jun 2020 23:00:23 +0000 (23:00 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 24 Jun 2020 23:00:23 +0000 (23:00 +0000)
https://bugs.webkit.org/show_bug.cgi?id=213568
<rdar://problem/64711653>

Reviewed by Darin Adler.

Source/WebCore:

Makes a couple of minor adjustments to allow `clipboard.write` and `clipboard.writeText` to work during a user-
triggered copy event (that is, from menu or key binding). See below for more details.

Test: editing/async-clipboard/clipboard-write-in-copy-event-handler.html

* Modules/async-clipboard/Clipboard.cpp:
(WebCore::shouldProceedWithClipboardWrite):

Add a mechanism to keep track of when Editor is handling a copy or cut event that is the result of interacting
with a key binding or menu action. Use this mechanism to grant write access via the async clipboard API.

* editing/Editor.cpp:
(WebCore::dispatchClipboardEvent):

Only attempt to commit the static pasteboard to the platform pasteboard during a copy event when preventing
default if the StaticPasteboard-backed DataTransfer actually contains data. This fixes a race condition when
writing to the pasteboard using the async clipboard API during the "copy" event, where the changeCount when
beginning to write is incremented due to the pasteboard getting cleared.

This change will also make it so that calling `preventDefault()` on the copy event will result in no change to
the system pasteboard, instead of clearing out any existing content on the pasteboard.

(WebCore::Editor::cut):
(WebCore::Editor::copy):
* editing/Editor.h:
(WebCore::Editor::isCopyingFromMenuOrKeyBinding const):
* editing/EditorCommand.cpp:
(WebCore::executeCopy):
(WebCore::executeCut):

LayoutTests:

Add a new layout test to exercise async clipboard writing API during the copy event.

* editing/async-clipboard/clipboard-write-in-copy-event-handler-expected.txt: Added.
* editing/async-clipboard/clipboard-write-in-copy-event-handler.html: Added.
* platform/win/TestExpectations:

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

LayoutTests/ChangeLog
LayoutTests/editing/async-clipboard/clipboard-write-in-copy-event-handler-expected.txt [new file with mode: 0644]
LayoutTests/editing/async-clipboard/clipboard-write-in-copy-event-handler.html [new file with mode: 0644]
LayoutTests/platform/win/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/Modules/async-clipboard/Clipboard.cpp
Source/WebCore/editing/Editor.cpp
Source/WebCore/editing/Editor.h
Source/WebCore/editing/EditorCommand.cpp

index 9a2b5bb..00da57f 100644 (file)
@@ -1,3 +1,17 @@
+2020-06-24  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Allow the async clipboard API to write data when copying via menu action or key binding
+        https://bugs.webkit.org/show_bug.cgi?id=213568
+        <rdar://problem/64711653>
+
+        Reviewed by Darin Adler.
+
+        Add a new layout test to exercise async clipboard writing API during the copy event.
+
+        * editing/async-clipboard/clipboard-write-in-copy-event-handler-expected.txt: Added.
+        * editing/async-clipboard/clipboard-write-in-copy-event-handler.html: Added.
+        * platform/win/TestExpectations:
+
 2020-06-24  Karl Rackler  <rackler@apple.com>
 
         Remove expectation for editing/spelling/editing-word-with-marker-1.html as they are passing. 
diff --git a/LayoutTests/editing/async-clipboard/clipboard-write-in-copy-event-handler-expected.txt b/LayoutTests/editing/async-clipboard/clipboard-write-in-copy-event-handler-expected.txt
new file mode 100644 (file)
index 0000000..2acb8c6
--- /dev/null
@@ -0,0 +1,10 @@
+This test verifies that clipboard items can be written while dispatching the 'copy' event. To manually run the test, copy the 'Copy me' text below.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS Successfully wrote to clipboard.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+Copy me
diff --git a/LayoutTests/editing/async-clipboard/clipboard-write-in-copy-event-handler.html b/LayoutTests/editing/async-clipboard/clipboard-write-in-copy-event-handler.html
new file mode 100644 (file)
index 0000000..8a8ac6f
--- /dev/null
@@ -0,0 +1,47 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true experimental:AsyncClipboardAPIEnabled=true  ] -->
+<html>
+    <meta charset="utf8">
+    <head>
+        <meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+        <script src="../../resources/js-test.js"></script>
+        <style>
+            button {
+                width: 100px;
+                padding: 1em;
+            }
+        </style>
+    </head>
+    <script>
+        jsTestIsAsync = true;
+
+        if (window.testRunner)
+            testRunner.setJavaScriptCanAccessClipboard(false);
+
+        description("This test verifies that clipboard items can be written while dispatching the 'copy' event. To manually run the test, copy the 'Copy me' text below.");
+
+        addEventListener("load", async function() {
+            const target = document.getElementById("target");
+
+            const textBlob = new Blob([ (new TextEncoder()).encode("Hello world") ], { type : "text/plain" });
+            target.addEventListener("copy", async event => {
+                try {
+                    event.preventDefault();
+                    await navigator.clipboard.write([ new ClipboardItem({ "text/plain" : textBlob })]);
+                    testPassed("Successfully wrote to clipboard.");
+                } catch (exception) {
+                    testFailed(`Failed with exception: ${exception}.`);
+                } finally {
+                    finishJSTest();
+                }
+            });
+
+            getSelection().selectAllChildren(target);
+
+            if (window.testRunner)
+                testRunner.execCommand("Copy");
+        });
+    </script>
+    <body>
+        <span id="target">Copy me</span>
+    </body>
+</html>
index f8bbb7b..3fcd220 100644 (file)
@@ -1202,6 +1202,7 @@ webkit.org/b/203100 editing/async-clipboard/clipboard-get-type-with-old-items.ht
 webkit.org/b/203100 editing/async-clipboard/clipboard-change-data-while-writing.html [ Skip ]
 webkit.org/b/203100 editing/async-clipboard/clipboard-write-basic.html [ Skip ]
 webkit.org/b/203100 editing/async-clipboard/clipboard-write-items-twice.html [ Skip ]
+webkit.org/b/203100 editing/async-clipboard/clipboard-write-in-copy-event-handler.html [ Skip ]
 webkit.org/b/203100 editing/async-clipboard/clipboard-write-text.html [ Skip ]
 webkit.org/b/203100 editing/async-clipboard/clipboard-do-not-read-text-from-platform-if-text-changes.html [ Skip ]
 webkit.org/b/203100 editing/async-clipboard/clipboard-read-text-from-platform.html [ Skip ]
index de9087d..ae6561d 100644 (file)
@@ -1,3 +1,41 @@
+2020-06-24  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Allow the async clipboard API to write data when copying via menu action or key binding
+        https://bugs.webkit.org/show_bug.cgi?id=213568
+        <rdar://problem/64711653>
+
+        Reviewed by Darin Adler.
+
+        Makes a couple of minor adjustments to allow `clipboard.write` and `clipboard.writeText` to work during a user-
+        triggered copy event (that is, from menu or key binding). See below for more details.
+
+        Test: editing/async-clipboard/clipboard-write-in-copy-event-handler.html
+
+        * Modules/async-clipboard/Clipboard.cpp:
+        (WebCore::shouldProceedWithClipboardWrite):
+
+        Add a mechanism to keep track of when Editor is handling a copy or cut event that is the result of interacting
+        with a key binding or menu action. Use this mechanism to grant write access via the async clipboard API.
+
+        * editing/Editor.cpp:
+        (WebCore::dispatchClipboardEvent):
+
+        Only attempt to commit the static pasteboard to the platform pasteboard during a copy event when preventing
+        default if the StaticPasteboard-backed DataTransfer actually contains data. This fixes a race condition when
+        writing to the pasteboard using the async clipboard API during the "copy" event, where the changeCount when
+        beginning to write is incremented due to the pasteboard getting cleared.
+
+        This change will also make it so that calling `preventDefault()` on the copy event will result in no change to
+        the system pasteboard, instead of clearing out any existing content on the pasteboard.
+
+        (WebCore::Editor::cut):
+        (WebCore::Editor::copy):
+        * editing/Editor.h:
+        (WebCore::Editor::isCopyingFromMenuOrKeyBinding const):
+        * editing/EditorCommand.cpp:
+        (WebCore::executeCopy):
+        (WebCore::executeCut):
+
 2020-06-24  Umar Iqbal  <uiqbal@apple.com>
 
         We should resurrect the older patch that collects some statistics of web API calls
index 8ea17eb..96fd091 100644 (file)
@@ -29,6 +29,7 @@
 #include "ClipboardImageReader.h"
 #include "ClipboardItem.h"
 #include "Document.h"
+#include "Editor.h"
 #include "Frame.h"
 #include "JSBlob.h"
 #include "JSClipboardItem.h"
@@ -49,7 +50,7 @@ WTF_MAKE_ISO_ALLOCATED_IMPL(Clipboard);
 static bool shouldProceedWithClipboardWrite(const Frame& frame)
 {
     auto& settings = frame.settings();
-    if (settings.javaScriptCanAccessClipboard())
+    if (settings.javaScriptCanAccessClipboard() || frame.editor().isCopyingFromMenuOrKeyBinding())
         return true;
 
     switch (settings.clipboardAccessPolicy()) {
index 3e36fa5..ca688ae 100644 (file)
@@ -432,7 +432,7 @@ static bool dispatchClipboardEvent(RefPtr<Element>&& target, ClipboardEventKind
 
     target->dispatchEvent(event);
     bool noDefaultProcessing = event->defaultPrevented();
-    if (noDefaultProcessing && (kind == ClipboardEventKind::Copy || kind == ClipboardEventKind::Cut))
+    if (noDefaultProcessing && (kind == ClipboardEventKind::Copy || kind == ClipboardEventKind::Cut) && dataTransfer->pasteboard().hasData())
         dataTransfer->commitToPasteboard(*Pasteboard::createForCopyAndPaste());
 
     dataTransfer->makeInvalidForSecurity();
@@ -1366,8 +1366,9 @@ bool Editor::insertParagraphSeparatorInQuotedContent()
     return true;
 }
 
-void Editor::cut()
+void Editor::cut(FromMenuOrKeyBinding fromMenuOrKeyBinding)
 {
+    SetForScope<bool> copyScope { m_copyingFromMenuOrKeyBinding, fromMenuOrKeyBinding == FromMenuOrKeyBinding::Yes };
     if (tryDHTMLCut())
         return; // DHTML did the whole operation
     if (!canCut()) {
@@ -1378,8 +1379,9 @@ void Editor::cut()
     performCutOrCopy(CutAction);
 }
 
-void Editor::copy()
+void Editor::copy(FromMenuOrKeyBinding fromMenuOrKeyBinding)
 {
+    SetForScope<bool> copyScope { m_copyingFromMenuOrKeyBinding, fromMenuOrKeyBinding == FromMenuOrKeyBinding::Yes };
     if (tryDHTMLCopy())
         return; // DHTML did the whole operation
     if (!canCopy()) {
index f8a9ad0..69d6059 100644 (file)
@@ -187,10 +187,10 @@ public:
     WEBCORE_EXPORT bool canSmartCopyOrDelete();
     bool shouldSmartDelete();
 
-    WEBCORE_EXPORT void cut();
-    WEBCORE_EXPORT void copy();
-
     enum class FromMenuOrKeyBinding : bool { No, Yes };
+    WEBCORE_EXPORT void cut(FromMenuOrKeyBinding = FromMenuOrKeyBinding::No);
+    WEBCORE_EXPORT void copy(FromMenuOrKeyBinding = FromMenuOrKeyBinding::No);
+
     WEBCORE_EXPORT void paste(FromMenuOrKeyBinding = FromMenuOrKeyBinding::No);
     void paste(Pasteboard&, FromMenuOrKeyBinding = FromMenuOrKeyBinding::No);
     WEBCORE_EXPORT void pasteAsPlainText(FromMenuOrKeyBinding = FromMenuOrKeyBinding::No);
@@ -574,6 +574,7 @@ public:
     WEBCORE_EXPORT void removeTextPlaceholder(TextPlaceholderElement&);
 
     bool isPastingFromMenuOrKeyBinding() const { return m_pastingFromMenuOrKeyBinding; }
+    bool isCopyingFromMenuOrKeyBinding() const { return m_copyingFromMenuOrKeyBinding; }
 
 private:
     Document& document() const { return m_document; }
@@ -658,6 +659,7 @@ private:
     bool m_editorUIUpdateTimerShouldCheckSpellingAndGrammar { false };
     bool m_editorUIUpdateTimerWasTriggeredByDictation { false };
     bool m_isHandlingAcceptedCandidate { false };
+    bool m_copyingFromMenuOrKeyBinding { false };
     bool m_pastingFromMenuOrKeyBinding { false };
 
 #if ENABLE(TELEPHONE_NUMBER_DETECTION) && PLATFORM(MAC)
index 9225d7d..d6ea426 100644 (file)
@@ -240,9 +240,9 @@ static bool executeBackColor(Frame& frame, Event*, EditorCommandSource source, c
     return executeApplyStyle(frame, source, EditAction::SetBackgroundColor, CSSPropertyBackgroundColor, value);
 }
 
-static bool executeCopy(Frame& frame, Event*, EditorCommandSource, const String&)
+static bool executeCopy(Frame& frame, Event*, EditorCommandSource source, const String&)
 {
-    frame.editor().copy();
+    frame.editor().copy(source == CommandFromMenuOrKeyBinding ? Editor::FromMenuOrKeyBinding::Yes : Editor::FromMenuOrKeyBinding::No);
     return true;
 }
 
@@ -260,7 +260,7 @@ static bool executeCut(Frame& frame, Event*, EditorCommandSource source, const S
 {
     if (source == CommandFromMenuOrKeyBinding) {
         UserTypingGestureIndicator typingGestureIndicator(frame);
-        frame.editor().cut();
+        frame.editor().cut(Editor::FromMenuOrKeyBinding::Yes);
     } else
         frame.editor().cut();
     return true;