Attachment 372386 Details for Bug 198922 – To land

[patch] To land

bug-198922-20190618144938.patch (text/plain), 9.08 KB, created by Daniel Bates on 2019-06-18 14:49:39 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Daniel Bates

Created: 2019-06-18 14:49:39 PDT

Size: 9.08 KB

patch

obsolete

>Subversion Revision: 246569
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 56a354fad1bae78c9369fa133ba631ea21f1b645..f17a8a4262d2b373c3c798d547f9d8f981967613 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -1,3 +1,29 @@
>+2019-06-18  Daniel Bates  <dabates@apple.com>
>+
>+        REGRESSION (r240757): Cannot dismiss the keyboard on http://apple.com/apple-tv-plus
>+        https://bugs.webkit.org/show_bug.cgi?id=198922
>+        <rdar://problem/50300056>
>+
>+        Reviewed by Wenson Hsieh.
>+
>+        Actually dismiss the keyboard as intended in r240757. Do not wait for the round-trip
>+        to the WebProcess to run through the -elementDidBlur steps in the UIProcess and hide
>+        the keyboard when a person explicitly dismisses the keyboard via the Done button (iPhone)
>+        or hide keyboard button (iPad).
>+
>+        Note that r240757 revealed another bug in this code, <https://bugs.webkit.org/show_bug.cgi?id=198928>.
>+        I am unclear of the implications of that bug, but it is clear for this bug that it
>+        never makes sense to round-trip to the WebProcess when the keyboard is dismissed by
>+        a user gesture.
>+
>+        * UIProcess/ios/WKContentViewInteraction.mm:
>+        (-[WKContentView endEditingAndUpdateFocusAppearanceWithReason:]): Invoke -_elementDidBlur
>+        to blur the element in the UIProcess and hide the keyboard.
>+        (-[WKContentView _elementDidBlur]): Prevent duplicate invocations of -didEndFormControlInteraction
>+        and setIsShowingInputViewForFocusedElement messages by only doing these actions when
>+        editablity changes. This covers the case where -_elementDidBlur may be invoked a second
>+        time (the reply in the round-trip). In that case we don't need to do these actions.
>+
> 2019-06-18  Alex Christensen  <achristensen@webkit.org>
> 
>         NetworkSession::networkStorageSession can return null
>diff --git a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>index 20fe6d4bb0c9bb98f581f2b0c0e90de6596e016d..5cace04be6e0891e9aec6914fa006339353aafdf 100644
>--- a/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>+++ b/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
>@@ -1226,8 +1226,12 @@ - (void)endEditingAndUpdateFocusAppearanceWithReason:(EndEditingReason)reason
>     if (!_webView._retainingActiveFocusedState) {
>         // We need to complete the editing operation before we blur the element.
>         [self _endEditing];
>-        if ((reason == EndEditingReasonAccessoryDone && !currentUserInterfaceIdiomIsPad()) || _keyboardDidRequestDismissal)
>+        if ((reason == EndEditingReasonAccessoryDone && !currentUserInterfaceIdiomIsPad()) || _keyboardDidRequestDismissal) {
>             _page->blurFocusedElement();
>+            // Don't wait for WebPageProxy::blurFocusedElement() to round-trip back to us to hide the keyboard
>+            // because we know that the user explicitly requested us to do so.
>+            [self _elementDidBlur];
>+        }
>     }
> 
>     [self _cancelInteraction];
>@@ -5282,12 +5286,13 @@ - (void)_elementDidBlur
>         [self removeFocusedFormControlOverlay];
> #endif
> 
>-    // The custom fixed position rect behavior is affected by -isFocusingElement, so if that changes we need to recompute rects.
>-    if (editableChanged)
>+    if (editableChanged) {
>+        // The custom fixed position rect behavior is affected by -isFocusingElement, so if that changes we need to recompute rects.
>         [_webView _scheduleVisibleContentRectUpdate];
> 
>-    [_webView didEndFormControlInteraction];
>-    _page->setIsShowingInputViewForFocusedElement(false);
>+        [_webView didEndFormControlInteraction];
>+        _page->setIsShowingInputViewForFocusedElement(false);
>+    }
> 
>     if (!_isChangingFocus)
>         _didAccessoryTabInitiateFocus = NO;
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index ca9606d9456dde36d5883e9fc44ba26817f0ea12..4665faec6184b7313e2186bfec4bf11a95abd599 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-06-18  Daniel Bates  <dabates@apple.com>
>+
>+        REGRESSION (r240757): Cannot dismiss the keyboard on http://apple.com/apple-tv-plus
>+        https://bugs.webkit.org/show_bug.cgi?id=198922
>+        <rdar://problem/50300056>
>+
>+        Reviewed by Wenson Hsieh.
>+
>+        Add a test to ensure that pressing Done hides the keyboard after tapping outside the focused
>+        element's frame.
>+
>+        * fast/events/ios/should-be-able-to-dismiss-form-accessory-after-tapping-outside-iframe-with-focused-field-expected.txt: Added.
>+        * fast/events/ios/should-be-able-to-dismiss-form-accessory-after-tapping-outside-iframe-with-focused-field.html: Added.
>+        * resources/ui-helper.js:
>+        (window.UIHelper.dismissFormAccessoryView): Added.
>+
> 2019-06-18  Alexey Shvayka  <shvaikalesh@gmail.com>
> 
>         [ESNExt] String.prototype.matchAll
>diff --git a/LayoutTests/fast/events/ios/should-be-able-to-dismiss-form-accessory-after-tapping-outside-iframe-with-focused-field-expected.txt b/LayoutTests/fast/events/ios/should-be-able-to-dismiss-form-accessory-after-tapping-outside-iframe-with-focused-field-expected.txt
>new file mode 100644
>index 0000000000000000000000000000000000000000..906fc2538b441a9b260bae61628f8f96c0e293f9
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/should-be-able-to-dismiss-form-accessory-after-tapping-outside-iframe-with-focused-field-expected.txt
>@@ -0,0 +1,13 @@
>+This test focuses the text field in the iframe, taps outside the iframe and then presses the Done button. The keyboard should hide.
>+
>+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
>+
>+
>+dispatched blur to main frame
>+dispatched focus to <input>
>+dispatched focus to main frame
>+PASS dismissed form accessory
>+PASS successfullyParsed is true
>+
>+TEST COMPLETE
>+
>diff --git a/LayoutTests/fast/events/ios/should-be-able-to-dismiss-form-accessory-after-tapping-outside-iframe-with-focused-field.html b/LayoutTests/fast/events/ios/should-be-able-to-dismiss-form-accessory-after-tapping-outside-iframe-with-focused-field.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..9d5d90aec96fb6bdb907673bd081d65bfcf6ec8b
>--- /dev/null
>+++ b/LayoutTests/fast/events/ios/should-be-able-to-dismiss-form-accessory-after-tapping-outside-iframe-with-focused-field.html
>@@ -0,0 +1,45 @@
>+<!DOCTYPE html>
>+<html>
>+<head>
>+<meta name="viewport" content="width=device-width">
>+<script src="../../../resources/js-test.js"></script>
>+<script src="../../../resources/ui-helper.js"></script>
>+</head>
>+<body>
>+<iframe id="frame" srcdoc="<style>body { margin: 0; padding: 0 }</style><input placeholder='Tap here'><script>window.parent.runTest()</script>"></iframe>
>+<script>
>+window.jsTestIsAsync = true;
>+
>+description("This test focuses the text field in the iframe, taps outside the iframe and then presses the Done button. The keyboard should hide.")
>+
>+async function runTest()
>+{
>+    let frame = document.getElementById("frame");
>+    let inputElementInFrame = frame.contentDocument.querySelector("input");
>+
>+    // 1. Add listeners
>+    window.addEventListener("focus", () => debug("dispatched focus to main frame"), { once: true });
>+    window.addEventListener("blur", () => debug("dispatched blur to main frame"), { once: true });
>+    inputElementInFrame.addEventListener("focus", () => debug("dispatched focus to &lt;input&gt;"), { once: true });
>+    inputElementInFrame.addEventListener("blur", () => debug("dispatched blur to &lt;input&gt;"), { once: true });
>+
>+    if (!window.testRunner)
>+        return;
>+
>+    // 2. Focus the <input> in the <iframe>.
>+    let x = frame.offsetLeft + inputElementInFrame.offsetLeft + Math.floor(inputElementInFrame.offsetWidth / 2);
>+    let y = frame.offsetTop + inputElementInFrame.offsetTop + Math.floor(inputElementInFrame.offsetHeight / 2);
>+    await UIHelper.activateAndWaitForInputSessionAt(x, y);
>+
>+    // 3. Tap somewhere in the main frame, then press the Done button.
>+    await UIHelper.activateAt(0, 0);
>+    await UIHelper.dismissFormAccessoryView(); // Press the Done button; ends the input session.
>+    await UIHelper.waitForKeyboardToHide();
>+
>+    testPassed("dismissed form accessory");
>+    document.body.removeChild(document.getElementById("frame"));
>+    finishJSTest();
>+}
>+</script>
>+</body>
>+</html>
>diff --git a/LayoutTests/resources/ui-helper.js b/LayoutTests/resources/ui-helper.js
>index 6aa67550eb1f22b13688e13bd48f2869b94e4215..5226b870dbdebfd7e3cabe2072cf96c746393298 100644
>--- a/LayoutTests/resources/ui-helper.js
>+++ b/LayoutTests/resources/ui-helper.js
>@@ -351,6 +351,20 @@ window.UIHelper = class UIHelper {
>         });
>     }
> 
>+    static dismissFormAccessoryView()
>+    {
>+        if (!this.isWebKit2() || !this.isIOSFamily())
>+            return Promise.resolve();
>+
>+        return new Promise(resolve => {
>+            testRunner.runUIScript(`
>+                (function() {
>+                    uiController.dismissFormAccessoryView();
>+                    uiController.uiScriptComplete();
>+                })()`, resolve);
>+        });
>+    }
>+
>     static isShowingKeyboard()
>     {
>         return new Promise(resolve => {

Actions: View | Formatted Diff | Diff

Attachments on bug 198922: 372253 | 372262 | 372268 | 372374 | 372386