[webkit-changes] [WebKit/WebKit] 5df770: Switching focus from a UITextField to an editable ...

Wenson Hsieh noreply at github.com
Wed Mar 22 07:44:39 PDT 2023 

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 5df7702c29e60e9fa8676902fc9658747b0f79a9
      https://github.com/WebKit/WebKit/commit/5df7702c29e60e9fa8676902fc9658747b0f79a9
  Author: Wenson Hsieh <wenson_hsieh at apple.com>
  Date:   2023-03-22 (Wed, 22 Mar 2023)

  Changed paths:
    A LayoutTests/fast/forms/ios/keyboard-stability-when-tapping-editable-element-expected.txt
    A LayoutTests/fast/forms/ios/keyboard-stability-when-tapping-editable-element.html
    M LayoutTests/resources/ui-helper.js
    M Source/WebKit/SourcesCocoa.txt
    M Source/WebKit/UIProcess/PageClient.h
    M Source/WebKit/UIProcess/WebPageProxy.h
    M Source/WebKit/UIProcess/WebPageProxy.messages.in
    R Source/WebKit/UIProcess/ios/InputViewUpdateDeferrer.h
    R Source/WebKit/UIProcess/ios/InputViewUpdateDeferrer.mm
    M Source/WebKit/UIProcess/ios/PageClientImplIOS.h
    M Source/WebKit/UIProcess/ios/PageClientImplIOS.mm
    M Source/WebKit/UIProcess/ios/WKContentView.mm
    M Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
    M Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
    M Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm
    M Source/WebKit/WebKit.xcodeproj/project.pbxproj
    M Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm
    M Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
    M Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
    M Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h
    M Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm
    M Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h
    M Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm

  Log Message:
  -----------
  Switching focus from a UITextField to an editable WKWebView causes the keyboard to dance
https://bugs.webkit.org/show_bug.cgi?id=254236
rdar://103712135

Reviewed by Aditya Keerthi.

Currently, when focusing editable web content while a native text field is focused, the keyboard
begins animated dismissal for a few frames, before appearing again. This happens because the web
view steals first responder away from the native text field upon tapping, underneath
`-_singleTapRecognized:`; however, we don't attempt to reload input views (and thus show the
keyboard) until the element focus IPC message arrives in the UI process (which is sent when
dispatching a synthetic click event). This is partly mitigated by 208968 at main and its predecessor,
186343 at main, which adopt UIKit SPI in order to keep the keyboard stable in this scenario. While this
works well in practice, it's still racy in some cases. Consider the following sequence of events:

(UI)    `-_singleTapRecognized:` is called, we become first responder, and begin holding onto the
        input view update deferral token. An activity state update is sent to the web process.

(WEB)   The potential tap is received, and we dispatch onto the runloop to complete the synthetic
        click (and dispatch click events).

(WEB)   The activity state is received, the web view becomes focused, and we tell the UI process
        that the activity state was received.

(WEB)   The synthetic click is dispatched, the editable element is focused, and we send
        `WebPageProxy::ElementDidFocus` to the UI process.

(UI)    The activity state change handler is invoked, causing us to relinquish the input view update
        deferral token. The keyboard begins animating away, since the web view isn't considered
        editable yet.

(UI)    `WebPageProxy::elementDidFocus()` is called, and we reload input views, causing the keyboard
        to cancel its hiding animation and reappear.

The longer the delay between the last two steps, the more noticeable the "keyboard dance" becomes.
To fix this, we refactor this code so that instead of taking a RAII token representing the scope of
input view update deferral, we instead maintain an `OptionSet` of flags (detailed below) indicating
different reasons for deferring input view updates; as long as any bit in this option set is set, we
defer input view updates.

Test: fast/forms/ios/keyboard-stability-when-tapping-editable-element.html

* Source/WebKit/SourcesCocoa.txt:
* Source/WebKit/UIProcess/PageClient.h:
* Source/WebKit/UIProcess/WebPageProxy.h:
* Source/WebKit/UIProcess/WebPageProxy.messages.in:
* Source/WebKit/UIProcess/ios/InputViewUpdateDeferrer.h: Removed.
* Source/WebKit/UIProcess/ios/InputViewUpdateDeferrer.mm: Removed.

Delete `InputViewUpdateDeferrer`, now that logic is all in `WKContentViewInteraction`.

* Source/WebKit/UIProcess/ios/PageClientImplIOS.h:
* Source/WebKit/UIProcess/ios/PageClientImplIOS.mm:
(WebKit::PageClientImpl::didHandleTapAsHover):

Add a new IPC message so that the web process can inform the UI process when a tap is handled as a
mouse hover, due to content change observation. See below for more details.

* Source/WebKit/UIProcess/ios/WKContentView.mm:
* Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:
* Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView cleanUpInteraction]):
(-[WKContentView _cancelPreviousResetInputViewDeferralRequest]):
(-[WKContentView _scheduleResetInputViewDeferralAfterBecomingFirstResponder]):
(-[WKContentView becomeFirstResponderForWebView]):
(-[WKContentView endEditingAndUpdateFocusAppearanceWithReason:]):
(-[WKContentView _commitPotentialTapFailed]):
(-[WKContentView _didNotHandleTapAsClick:]):
(-[WKContentView _didHandleTapAsHover]):
(-[WKContentView _didCompleteSyntheticClick]):
(-[WKContentView _singleTapRecognized:]):
(-[WKContentView startDeferringInputViewUpdates:]):
(-[WKContentView stopDeferringInputViewUpdates:]):
(-[WKContentView stopDeferringInputViewUpdatesForAllSources]):
(-[WKContentView _elementDidFocus:userIsInteracting:blurPreviousNode:activityStateChanges:userObject:]):
(-[WKContentView _resetInputViewDeferral]): Deleted.

Adjust code that previously relied on `InputViewUpdateDeferrer` to instead add or remove
`InputViewUpdateDeferralSources`. There are three distinct sources:

•   `BecomeFirstResponder`: this is first set when becoming first responder in `WKContentView`, and
    is unset once the activity state change (as a result of becoming first responder) has been
    handled by the web process. It's also unset after the next `ElementDidFocus` message arrives in
    the UI process.

•   `TapGesture`: this is set when handling a tap over web content, right before committing the
    potential tap to the web page; it's unset once any of the following are invoked:
    `_commitPotentialTapFailed`, `_didNotHandleTapAsClick:`, or `_didCompleteSyntheticClick`.

•   `ChangingFocusedElement`: this is set only in the case where we're immediately changing focus
    from one element to another underneath `-_elementDidFocus:…:`, and is immediately unset once we
    exit that method.

* Source/WebKit/UIProcess/ios/WebPageProxyIOS.mm:
(WebKit::WebPageProxy::didHandleTapAsHover):
* Source/WebKit/WebKit.xcodeproj/project.pbxproj:
* Source/WebKit/WebProcess/WebPage/ios/WebPageIOS.mm:
(WebKit::WebPage::handleSyntheticClick):
(WebKit::WebPage::didFinishContentChangeObserving):

Update these to send `WebPageProxy::DidHandleTapAsHover` in the case where the content change
observer causes us to handle the tap as mouse hover, to ensure that we don't leave the input view
update deferral flag  Also, drive-by fix a typo that I stumbled into
while debugging this code.

* LayoutTests/fast/forms/ios/keyboard-stability-when-tapping-editable-element-expected.txt: Added.
* LayoutTests/fast/forms/ios/keyboard-stability-when-tapping-editable-element.html: Added.
* LayoutTests/resources/ui-helper.js:
(window.UIHelper.keyboardWillHideCount):
* Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:
(-[WKContentView startDeferringInputViewUpdates:]):
(-[WKContentView stopDeferringInputViewUpdates:]):
(-[WKContentView stopDeferringInputViewUpdatesForAllSources]):
* Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
* Tools/TestRunnerShared/UIScriptContext/UIScriptController.h:
(WTR::UIScriptController::keyboardWillHideCount const):
* Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h:
* Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:
(-[TestRunnerWKWebView initWithFrame:configuration:]):
(-[TestRunnerWKWebView _keyboardWillHide]):
* Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h:
* Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptControllerIOS::keyboardWillHideCount const):

Add a new layout test to exercise the change, along with a new testing hook that allows us to check
whether or not there was an attempt to hide the keyboard between any two given points in time (by
simply keeping a tally of the number of times the `KeyboardWillHide` notification was received).

Canonical link: https://commits.webkit.org/261964@main

More information about the webkit-changes mailing list