[webkit-changes] [WebKit/WebKit] 8796ae: REGRESSION (268955 at main): Accepting inline predict...

Wenson Hsieh noreply at github.com
Thu Nov 30 16:02:42 PST 2023


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 8796ae7158aac06497a247100c5e91ca8e25ae11
      https://github.com/WebKit/WebKit/commit/8796ae7158aac06497a247100c5e91ca8e25ae11
  Author: Wenson Hsieh <wenson_hsieh at apple.com>
  Date:   2023-11-30 (Thu, 30 Nov 2023)

  Changed paths:
    A LayoutTests/editing/selection/ios/tap-to-change-selection-after-accepting-inline-prediction-expected.txt
    A LayoutTests/editing/selection/ios/tap-to-change-selection-after-accepting-inline-prediction.html
    M LayoutTests/resources/ui-helper.js
    M Source/WebKit/UIProcess/ios/UIKitUtilities.h
    M Source/WebKit/UIProcess/ios/UIKitUtilities.mm
    M Source/WebKit/UIProcess/ios/WKContentViewInteraction.h
    M Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm
    M Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
    M Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
    M Tools/TestRunnerShared/spi/UIKitSPIForTesting.h
    M Tools/WebKitTestRunner/TestOptions.cpp
    M Tools/WebKitTestRunner/TestOptions.h
    M Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm
    M Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h
    M Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm

  Log Message:
  -----------
  REGRESSION (268955 at main): Accepting inline predictions breaks ability to change selection by tapping in Mail compose
https://bugs.webkit.org/show_bug.cgi?id=265592
rdar://118094413

Reviewed by Tim Horton and Abrar Rahman Protyasha.

After the changes in 268955 at main, we sometimes end up with `_textInteractionLoupeGestureRecognizer`
and `_textInteractionTapGestureRecognizer` ivars that point to old gesture recognizers that are no
longer installed on the content view.

While we currently refresh these two references every time we call `-setUpTextSelectionAssistant`
(i.e. when we show or hide the keyboard, or change the focused element), it's actually possible for
UIKit to also add or remove these text interaction gestures from the content view; one of the ways
in which this may happen is upon accepting an inline prediction in Mail compose, which causes us to
get into a state where the text interaction multi-tap gesture is swapped out from underneath us,
subsequently causing `-gestureRecognizer:shouldRecognizeSimultaneouslyWithGestureRecognizer:` to no
longer return `YES` in the case of the synthetic click and text tap gestures. This leads to the
synthetic click taking precedence, causing single taps to try and change the selection to fail.

To fix this, we replace these ivars with properties on the content view, which internally check if
the last cached gesture is still installed on the content view; if not, we find the new gesture and
cache it.

Test: editing/selection/ios/tap-to-change-selection-after-accepting-inline-prediction.html

* LayoutTests/editing/selection/ios/tap-to-change-selection-after-accepting-inline-prediction-expected.txt: Added.
* LayoutTests/editing/selection/ios/tap-to-change-selection-after-accepting-inline-prediction.html: Added.

Add a new layout test to exercise the change.

* LayoutTests/resources/ui-helper.js:
(window.UIHelper.async setInlinePrediction):
(window.UIHelper.async acceptInlinePrediction):

Add test infrastructure for both setting and accepting inline predictions, relative to the current
selection.

* Source/WebKit/UIProcess/ios/UIKitUtilities.h:
* Source/WebKit/UIProcess/ios/UIKitUtilities.mm:
(-[UIGestureRecognizer _wk_isTextInteractionLoupeGesture]):
(-[UIGestureRecognizer _wk_isTextInteractionTapGesture]):

Add a couple of utility functions to return whether or not a given gesture recognizer is a text
interaction multi-tap or loupe gesture.

* Source/WebKit/UIProcess/ios/WKContentViewInteraction.h:
* Source/WebKit/UIProcess/ios/WKContentViewInteraction.mm:

Replace uses of `_textInteractionTapGestureRecognizer` and `_textInteractionLoupeGestureRecognizer`
with the new properties instead, and rename these to `_cachedTextInteraction*GestureRecognizer` to
make it clear that these ivars may not be up to date. See above for more details.

In places where we already have a `UIGestureRecognizer` instance and we're checking whether or not
it's a loupe or tap gesture, we instead call into the category properties directly, to avoid
unnecessarily scanning the gesture recognizer list.

(-[WKContentView gestureRecognizer:canBePreventedByGestureRecognizer:]):
(-[WKContentView textInteractionLoupeGestureRecognizer]):
(-[WKContentView textInteractionTapGestureRecognizer]):
(-[WKContentView gestureRecognizer:shouldRecognizeSimultaneouslyWithGestureRecognizer:]):
(-[WKContentView textInteractionGesture:shouldBeginAtPoint:]):
(-[WKContentView setUpTextSelectionAssistant]):
(logTextInteraction):

Additionally, rename `logTextInteractionAssistantSelectionChange` to just `logTextInteraction`, now
that it doesn't take a text interaction assistant.

(-[WKContentView changeSelectionWithGestureAt:withGesture:withState:withFlags:]):
(-[WKContentView changeSelectionWithTouchAt:withSelectionTouch:baseIsStart:withFlags:]):
(-[WKContentView changeSelectionWithTouchesFrom:to:withGesture:withState:]):
(-[WKContentView _selectPositionAtPoint:stayingWithinFocusedElement:completionHandler:]):
(-[WKContentView selectPositionAtBoundary:inDirection:fromPoint:completionHandler:]):
(logTextInteractionAssistantSelectionChange): Deleted.
(-[WKContentView moveSelectionAtBoundary:inDirection:completionHandler:]):
(-[WKContentView selectTextWithGranularity:atPoint:completionHandler:]):
(-[WKContentView beginSelectionInDirection:completionHandler:]):
(-[WKContentView updateSelectionWithExtentPoint:completionHandler:]):
(-[WKContentView updateSelectionWithExtentPoint:withBoundary:completionHandler:]):
(-[WKContentView shouldDeferGestureDueToImageAnalysis:]):
(-[WKContentView deferringGestureRecognizer:shouldDeferOtherGestureRecognizer:]):
(-[WKContentView cancelActiveTextInteractionGestures]):
(-[WKContentView selectPositionAtPoint:withContextRequest:completionHandler:]):
* Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
* Tools/TestRunnerShared/UIScriptContext/UIScriptController.h:
(WTR::UIScriptController::setInlinePrediction):
(WTR::UIScriptController::acceptInlinePrediction):
* Tools/TestRunnerShared/spi/UIKitSPIForTesting.h:
* Tools/WebKitTestRunner/TestOptions.cpp:
(WTR::TestOptions::defaults):
(WTR::TestOptions::keyTypeMapping):
* Tools/WebKitTestRunner/TestOptions.h:
(WTR::TestOptions::allowsInlinePredictions const):
* Tools/WebKitTestRunner/cocoa/TestControllerCocoa.mm:
(WTR::TestController::configureWebpagePreferences):
* Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h:
* Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptControllerIOS::setInlinePrediction):
(WTR::UIScriptControllerIOS::acceptInlinePrediction):

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




More information about the webkit-changes mailing list