Attachment 369441 Details for Bug 197632 – Patch

[patch] Patch

bug-197632-20190508161409.patch (text/plain), 14.97 KB, created by Daniel Bates on 2019-05-08 16:14:09 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Daniel Bates

Created: 2019-05-08 16:14:09 PDT

Size: 14.97 KB

patch

obsolete

>Subversion Revision: 244742
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index adab64b456fa783e77699ad0b5c7e0d610b6657c..e65217b996dc19162cf6b1c63e6cf74b696c6790 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,45 @@
>+2019-05-08  Daniel Bates  <dabates@apple.com>
>+
>+        [iOS] Unable to commit search on MSN.com, qq.com, or sina.com.cn using enter key (hardware or software keyboard)
>+        https://bugs.webkit.org/show_bug.cgi?id=197632
>+        <rdar://problem/47902054>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Fixes an issue where it is not possible to submit a <form> with target = "_blank", that is,
>+        a form that opens a new window.
>+
>+        By default we only allow popups to open if they were user initiated (like a person clicked on
>+        a link). We achieve this by putting a token on the stack, called the UserGestureToken, when
>+        WebCore processes an event from WebKit. So long as this token is on the stack we consider
>+        all requests to open a popup to be user initiated. And we implicity submit a form when pressing
>+        the Return key in an HTML input element during the processing of a TextInputEvent dispatched as
>+        part of inserting a \n into the field. On Mac, the keydown dispatches a TextInputEvent synchronously.
>+        However on iOS text insertion, and hence a dispatch of a TextInputEvent, occurs asynchronously
>+        with respect to the keydown event. So, by the time the UI process calls back to the WebProcess
>+        to perform the text insertion of '\n' we have long since popped the UserGestureToken off the stack
>+        and hence we disallow opening a popup. To fix when -insertText is called we query the keyboard
>+        to determine if its being called by the keyboard. If it is then we can assume that this is
>+        part of key event handling and hence was initiated by the user. We can pass along this detail
>+        to the WebProcess for it to take out a push a new UserGestureToken onto the stack.
>+
>+        For now we only track whether text inserted by the keyboard was user initiated or not. In
>+        <https://bugs.webkit.org/show_bug.cgi?id=197721> we will fix this up for all editing commands.
>+
>+        * Platform/spi/ios/UIKitSPI.h: Expose SPI.
>+        * Shared/Cocoa/InsertTextOptions.cpp:
>+        (IPC::ArgumentCoder<WebKit::InsertTextOptions>::encode):
>+        (IPC::ArgumentCoder<WebKit::InsertTextOptions>::decode):
>+        Encode and decode whether we are processing a user gesture.
>+
>+        * Shared/Cocoa/InsertTextOptions.h:
>+        * UIProcess/ios/WKContentViewInteraction.mm:
>+        (-[WKContentView insertText:]): Query the keyboard to determine whether it called us or
>+        the embedding client did. We only want to privilege user initiated actions (the keyboard).
>+        * WebProcess/WebPage/WebPage.cpp:
>+        (WebKit::WebPage::insertTextAsync): Push a UserGestureToken onto the stack that is initialized
>+        depending on whether we are or are not processing a user gesture.
>+
> 2019-05-08  Daniel Bates  <dabates@apple.com>
> 
>         Pass insertTextAsync options as a struct
>diff --git a/Source/WebKit/Platform/spi/ios/UIKitSPI.h b/Source/WebKit/Platform/spi/ios/UIKitSPI.h
>index c262b37ff81d23eed22b46b2cd2129a16eed2c71..fabe18a70ce9bd47853234c76aeec593e09ef092 100644
>--- a/Source/WebKit/Platform/spi/ios/UIKitSPI.h
>+++ b/Source/WebKit/Platform/spi/ios/UIKitSPI.h
>@@ -1126,6 +1126,7 @@ typedef NS_OPTIONS(NSInteger, UIWKDocumentRequestFlags) {
> - (BOOL)handleKeyTextCommandForCurrentEvent;
> - (BOOL)handleKeyAppCommandForCurrentEvent;
> - (BOOL)handleKeyInputMethodCommandForCurrentEvent;
>+- (BOOL)isCallingInputDelegate;
> @property (nonatomic, readonly) UIKeyboardInputMode *currentInputModeInPreference;
> @end
> 
>diff --git a/Source/WebKit/Shared/Cocoa/InsertTextOptions.cpp b/Source/WebKit/Shared/Cocoa/InsertTextOptions.cpp
>index 4000efdc73baf6279bfa42e3c5e85c5efb437962..816442612a2279a8c33bd5cf3a8e2403746750be 100644
>--- a/Source/WebKit/Shared/Cocoa/InsertTextOptions.cpp
>+++ b/Source/WebKit/Shared/Cocoa/InsertTextOptions.cpp
>@@ -32,6 +32,7 @@ void ArgumentCoder<WebKit::InsertTextOptions>::encode(Encoder& encoder, const We
> {
>     encoder << options.registerUndoGroup;
>     encoder << options.suppressSelectionUpdate;
>+    encoder << options.processingUserGesture;
>     encoder << options.editingRangeIsRelativeTo;
> }
> 
>@@ -42,6 +43,8 @@ Optional<WebKit::InsertTextOptions> ArgumentCoder<WebKit::InsertTextOptions>::de
>         return WTF::nullopt;
>     if (!decoder.decode(options.suppressSelectionUpdate))
>         return WTF::nullopt;
>+    if (!decoder.decode(options.processingUserGesture))
>+        return WTF::nullopt;
>     if (!decoder.decode(options.editingRangeIsRelativeTo))
>         return WTF::nullopt;
>     return options;
>diff --git a/Source/WebKit/Shared/Cocoa/InsertTextOptions.h b/Source/WebKit/Shared/Cocoa/InsertTextOptions.h
>index b169520ae305fb679a7b8e31af57bbf1418e3f97..cd201ab2bad3d293df8b3ea0387e341ad4b70b51 100644
>--- a/Source/WebKit/Shared/Cocoa/InsertTextOptions.h
>+++ b/Source/WebKit/Shared/Cocoa/InsertTextOptions.h
>@@ -33,6 +33,7 @@ namespace WebKit {
> struct InsertTextOptions {
>     bool registerUndoGroup { false };
>     bool suppressSelectionUpdate { false };
>+    bool processingUserGesture { false };
>     EditingRangeIsRelativeTo editingRangeIsRelativeTo { EditingRangeIsRelativeTo::EditableRoot };
> };
> 
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index 03d24f631e8bacaa84ba76f857cdefb82c723837..a246c0c3c44c2780b2decf1a701567b12617cad0 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -4152,7 +4152,12 @@ - (void)deleteBackward
> // Inserts the given string, replacing any selected or marked text.
> - (void)insertText:(NSString *)aStringValue
> {
>-    _page->insertTextAsync(aStringValue, WebKit::EditingRange(), { });
>+    auto* keyboard = [UIKeyboardImpl sharedInstance];
>+
>+    WebKit::InsertTextOptions options;
>+    options.processingUserGesture = [keyboard respondsToSelector:@selector(isCallingInputDelegate)] && [UIKeyboardImpl sharedInstance].isCallingInputDelegate;
>+
>+    _page->insertTextAsync(aStringValue, WebKit::EditingRange(), WTFMove(options));
> }
> 
> - (BOOL)hasText
>@@ -4636,6 +4641,9 @@ - (void)keyboardScrollViewAnimatorDidFinishScrolling:(WKKeyboardScrollViewAnimat
> 
> - (void)executeEditCommandWithCallback:(NSString *)commandName
> {
>+    // FIXME: Editing commands are not considered by WebKit as user initiated even if they are the result
>+    // of keydown or keyup. We need to query the keyboard to determine if this was called from the keyboard
>+    // or not to know whether to tell WebKit to treat this command as user initiated or not.
>     [self beginSelectionChange];
>     RetainPtr<WKContentView> view = self;
>     _page->executeEditCommand(commandName, { }, [view](WebKit::CallbackBase::Error) {
>diff --git a/Source/WebKit/WebProcess/WebPage/WebPage.cpp b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>index c27f5b7718a149fb0d68011ff5ed9f5e8c642cf7..3283b376b1bbc2a8db011b2152710f72cfeae094 100644
>--- a/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>+++ b/Source/WebKit/WebProcess/WebPage/WebPage.cpp
>@@ -5125,6 +5125,8 @@ void WebPage::insertTextAsync(const String& text, const EditingRange& replacemen
> 
>     Ref<Frame> protector(frame);
> 
>+    UserGestureIndicator gestureIndicator { options.processingUserGesture ? ProcessingUserGesture : NotProcessingUserGesture, frame.document() };
>+
>     bool replacesText = false;
>     if (replacementEditingRange.location != notFound) {
>         if (auto replacementRange = EditingRange::toRange(frame, replacementEditingRange, options.editingRangeIsRelativeTo)) {
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 6b34855d3ca555886180ece68496ec03b28c215c..7065ee1eec5a49abc8a85ceb46f02a611d6c8ee9 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,21 @@
>+2019-05-08  Daniel Bates  <dabates@apple.com>
>+
>+        [iOS] Unable to commit search on MSN.com, qq.com, or sina.com.cn using enter key (hardware or software keyboard)
>+        https://bugs.webkit.org/show_bug.cgi?id=197632
>+        <rdar://problem/47902054>
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Add tests to ensure we fire input and keypress events in the correct order and that we can
>+        submit a <form> with target = "_blank" using the Return key.
>+
>+        * fast/events/ios/fire-input-and-keypress-on-return-key-expected.txt: Added.
>+        * fast/events/ios/fire-input-and-keypress-on-return-key.html: Added.
>+        * fast/events/ios/submit-form-target-blank-using-return-key-expected.txt: Added.
>+        * fast/events/ios/submit-form-target-blank-using-return-key.html: Added.
>+        * platform/ios/TestExpectations: Skip the test until we have the UIKit SPI added
>+        in <rdar://problem/50596032>.
>+
> 2018-12-05  Daniel Bates  <dabates@apple.com>
> 
>         [iOS] Add test to ensure that a web page can prevent the default for Command + A
>diff --git a/LayoutTests/fast/events/ios/fire-input-and-keypress-on-return-key-expected.txt b/LayoutTests/fast/events/ios/fire-input-and-keypress-on-return-key-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..163ae61cdd0682b41a62c34bc793967109d69c54
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/fire-input-and-keypress-on-return-key-expected.txt
>@@ -0,0 +1,11 @@
>+Tests that pressing the Return key in a content editable elemnent dispatches a DOM input event and DOM keypress event (in that order). To run this test by hand, focus the content editable element below and press the Return key.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS event.key is "Enter"
>+PASS event.inputType is "insertParagraph"
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/events/ios/fire-input-and-keypress-on-return-key.html b/LayoutTests/fast/events/ios/fire-input-and-keypress-on-return-key.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..892291ce542f1b3b3bff1e868fe6a7141e90961e
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/fire-input-and-keypress-on-return-key.html
>@@ -0,0 +1,49 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script src="../../../resources/js-test.js"></script>
>+<script src="../../../resources/ui-helper.js"></script>
>+</head>
>+<body>
>+<p id="description"></p>
>+<div id="test" contenteditable="true" style="width:256px; height: 256px; border: 1px solid black"></div>
>+<div id="console"></div>
>+<script>
>+window.jsTestIsAsync = true;
>+
>+function done()
>+{
>+    document.body.removeChild(document.getElementById("test"));
>+    finishJSTest();
>+}
>+
>+function checkInputEvent()
>+{
>+    shouldBeEqualToString("event.inputType", "insertParagraph");
>+    done();
>+}
>+
>+function checkKeypressAndDone()
>+{
>+    shouldBeEqualToString("event.key", "Enter");
>+}
>+
>+function runTest()
>+{
>+    function handleFocus(event) {
>+        event.target.addEventListener("input", checkInputEvent, { once: true });
>+        event.target.addEventListener("keypress", checkKeypressAndDone, { once: true });
>+        if (window.testRunner)
>+            UIHelper.keyDown("return");
>+    }
>+    let test = document.getElementById("test");
>+    test.addEventListener("focus", handleFocus, { once: true });
>+    if (window.testRunner)
>+        UIHelper.activateElement(test);
>+}
>+
>+description("Tests that pressing the Return key in a content editable elemnent dispatches a DOM input event and DOM keypress event (in that order). To run this test by hand, focus the content editable element below and press the <key>Return<key> key.");
>+runTest();
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/fast/events/ios/submit-form-target-blank-using-return-key-expected.txt b/LayoutTests/fast/events/ios/submit-form-target-blank-using-return-key-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..18183b82100ee3f88c30a7beebec9e3eb1be31e7
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/submit-form-target-blank-using-return-key-expected.txt
>@@ -0,0 +1,10 @@
>+Tests that pressing the Return key in a text field with an associated form implicitly submits the form. To run this test by hand, focus the text field below and press the Return key.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+PASS event.key is "Enter"
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/events/ios/submit-form-target-blank-using-return-key.html b/LayoutTests/fast/events/ios/submit-form-target-blank-using-return-key.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..f918dc4417caa588fad78e5a811ed6bddb98d3c9
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/submit-form-target-blank-using-return-key.html
>@@ -0,0 +1,57 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<script src="../../../resources/js-test.js"></script>
>+<script src="../../../resources/ui-helper.js"></script>
>+</head>
>+<body>
>+<p id="description"></p>
>+<form action="resources/submit-form-target-blank-using-return-key.html" target="_blank">
>+    <input type="text" id="test">
>+</form>
>+<div id="console"></div>
>+<script>
>+window.jsTestIsAsync = true;
>+
>+if (window.testRunner) {
>+    testRunner.setCanOpenWindows(true);
>+    testRunner.setCloseRemainingWindowsWhenComplete(true);
>+    testRunner.setPopupBlockingEnabled(true);
>+}
>+
>+function done()
>+{
>+    document.body.removeChild(document.querySelector("form"));
>+    finishJSTest();
>+}
>+
>+function checkInputEvent()
>+{
>+    testFailed("Should not have dispatched input event.");
>+    done();
>+}
>+
>+function checkKeypressAndDone()
>+{
>+    shouldBeEqualToString("event.key", "Enter");
>+}
>+
>+function runTest()
>+{
>+    function handleFocus(event) {
>+        event.target.addEventListener("input", checkInputEvent, { once: true });
>+        event.target.addEventListener("keypress", checkKeypressAndDone, { once: true });
>+        if (window.testRunner)
>+            UIHelper.keyDown("return");
>+    }
>+    let test = document.getElementById("test");
>+    test.addEventListener("focus", handleFocus, { once: true });
>+    if (window.testRunner)
>+        UIHelper.activateElement(test);
>+}
>+
>+description("Tests that pressing the Return key in a text field with an associated form implicitly submits the form. To run this test by hand, focus the text field below and press the <key>Return<key> key.");
>+runTest();
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/platform/ios/TestExpectations b/LayoutTests/platform/ios/TestExpectations
>index 1c967b9d97c3c943c38fc0768d70ee1d80b251d3..013b281207f2c4b7dbe498a979c0a885d0642e6f 100644
>--- a/LayoutTests/platform/ios/TestExpectations
>+++ b/LayoutTests/platform/ios/TestExpectations
>@@ -3260,3 +3260,6 @@ webkit.org/b/191711 quicklook/numbers.html [ ImageOnlyFailure ]
> 
> # FIXME: Unskip the following test once we have the fix for <rdar://problem/46430796>.
> fast/events/ios/key-command-select-all-prevent-default.html [ Skip ]
>+
>+# FIXME: Unskip the following test once we have the fix for <rdar://problem/50596032>.
>+fast/events/ios/submit-form-target-blank-using-return-key.html

Actions: View | Formatted Diff | Diff

Attachments on bug 197632: 369189 | 369277 | 369428 | 369441 | 369446 | 369464 | 369497