Changeset 246570 in webkit

Timestamp:

Jun 18, 2019 2:50:20 PM (5 years ago)

Author:

dbates@webkit.org

Message:

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.

Source/WebKit:

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.

LayoutTests:

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/events/ios/should-be-able-to-dismiss-form-accessory-after-tapping-outside-iframe-with-focused-field-expected.txt (added)
• LayoutTests/fast/events/ios/should-be-able-to-dismiss-form-accessory-after-tapping-outside-iframe-with-focused-field.html (added)
• LayoutTests/resources/ui-helper.js (modified) (1 diff)
• Source/WebKit/ChangeLog (modified) (1 diff)
• Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+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>
 

trunk/LayoutTests/resources/ui-helper.js

@@ -352 +352 @@
     }
 
+    static dismissFormAccessoryView()
+    {
+        if (!this.isWebKit2() || !this.isIOSFamily())
+            return Promise.resolve();
+
+        return new Promise(resolve => {
+            testRunner.runUIScript(`
+                (function() {
+                    uiController.dismissFormAccessoryView();
+                    uiController.uiScriptComplete();
+                })()`, resolve);
+        });
+    }
+
     static isShowingKeyboard()
     {

trunk/Source/WebKit/ChangeLog

@@ +1 @@
+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>
 

trunk/Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm

@@ -1227 +1227 @@
         // 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];
+        }
     }
 
@@ -5283 +5287 @@
 #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)