[webkit-changes] [WebKit/WebKit] 83b571: Move off of UIKit SPI: -[UIScrollView _zoomToCente...

Tue Aug 22 17:12:00 PDT 2023

  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 83b571eca61c5eb2becd4a6f883e2c6ef2bf6ae2
      https://github.com/WebKit/WebKit/commit/83b571eca61c5eb2becd4a6f883e2c6ef2bf6ae2
  Author: Wenson Hsieh <wenson_hsieh at apple.com>
  Date:   2023-08-22 (Tue, 22 Aug 2023)

  Changed paths:
    M LayoutTests/fast/forms/ios/accessory-bar-navigation-expected.txt
    M LayoutTests/fast/forms/ios/accessory-bar-navigation.html
    M LayoutTests/fast/forms/ios/focus-input-via-button-no-scaling-expected.txt
    M LayoutTests/fast/forms/ios/focus-input-via-button-no-scaling.html
    M LayoutTests/fast/forms/ios/resources/zooming-test-utils.js
    M LayoutTests/fast/forms/ios/scroll-to-reveal-focused-select-expected.txt
    M LayoutTests/fast/forms/ios/scroll-to-reveal-focused-select.html
    M LayoutTests/fast/forms/ios/user-scalable-does-not-scale-for-keyboard-focus-with-author-defined-scale-expected.txt
    M LayoutTests/fast/forms/ios/user-scalable-does-not-scale-for-keyboard-focus-with-author-defined-scale.html
    M LayoutTests/fast/forms/ios/user-scalable-does-not-scale-for-keyboard-focus-with-user-scalable-no-expected.txt
    M LayoutTests/fast/forms/ios/user-scalable-does-not-scale-for-keyboard-focus-with-user-scalable-no.html
    M LayoutTests/fast/shadow-dom/ios/accessory-bar-work-on-input-with-tabindex-in-shadow-tree.html
    M LayoutTests/platform/ios-wk2/TestExpectations
    R LayoutTests/platform/ios/fast/forms/ios/accessory-bar-navigation-expected.txt
    R LayoutTests/platform/ios/fast/forms/ios/focus-input-via-button-no-scaling-expected.txt
    M LayoutTests/resources/ui-helper.js
    M Source/WebKit/Platform/spi/ios/UIKitSPI.h
    M Source/WebKit/UIProcess/API/ios/WKWebViewIOS.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:
  -----------
  Move off of UIKit SPI: -[UIScrollView _zoomToCenter:scale:duration:force:]
https://bugs.webkit.org/show_bug.cgi?id=260447
rdar://114162044

Reviewed by Aditya Keerthi.

Replace current uses of the scroll view SPI `-_zoomToCenter:scale:duration:force:` with an API
equivalent, `-zoomToRect:animated:`. This is used for two purposes in WebKit: handling double-tap to
zoom, and scrolling to reveal the focused element (or selection rect, if appropriate) in the center
of the unobscured viewport, accounting for the keyboard height. This is *largely* a drop-in
replacement, but with several caveats:

1.  Since it only takes a rect, we need to pass in an appropriately-sized rect, derived from the
    given target scale and target point by expanding outward from the center. UIKit takes care of
    clipping to visible bounds along the edges of the scroll view.

2.  If `-zoomToRect:animated:` is called with a rect that results in no change in zoom scale, the
    scrolling animation speed defaults to 100 ms, which is more than twice as fast as the current
    animation speed of 250 ms when scrolling to reveal a focused form control, which results in a
    jarring animation. To work around this and preserve existing behavior, we pass in `animated:NO`
    only for the case where the scale isn't changing, but then wrap the whole call inside of
    `+[UIView animateWithDuration:animations:]`, with the animation duration set to 250 ms (i.e.
    consistent with the behavior we've always shipped, when focusing form controls).

3.  Additionally, in the above case where `-zoomToRect:animated:` only scrolls (and doesn't change
    the zoom scale), the scroll view delegate methods for `-scrollViewWillBeginZooming:withView:`
    and `-scrollViewDidEndZooming:withView:atScale:` will never get called. This is not the case
    currently, since we're able to pass the `force:YES` when triggering the zoom. This breaks a
    number of layout tests, which rely on test runner's `didEndZoomingCallback` to fire after
    focusing a text field, even if the view doesn't actually change zoom scale. These tests are:

    • fast/forms/ios/accessory-bar-navigation.html
    • fast/forms/ios/focus-input-via-button-no-scaling.html
    • fast/forms/ios/scroll-to-reveal-focused-select.html
    • fast/forms/ios/user-scalable-does-not-scale-for-keyboard-focus-with-author-defined-scale.html
    • fast/forms/ios/user-scalable-does-not-scale-for-keyboard-focus-with-user-scalable-no.html

    ...to prevent these tests from timing out, we introduce new testing helpers to wait for
    all scrolling and zooming animations on the main `WKScrollView` to end; this allows us to easily
    wait for the focused element to be revealed with something like:

    ```
    await UIHelper.activateElementAndWaitForInputSession(someTextField);
    await UIHelper.waitForZoomingOrScrollingToEnd();

    let finalScale = await UIHelper.zoomScale();
    // check finalScale ...
    ```

    ...instead of having to write and invoke custom UI-side script.

* LayoutTests/fast/forms/ios/accessory-bar-navigation-expected.txt:
* LayoutTests/fast/forms/ios/accessory-bar-navigation.html:
* LayoutTests/fast/forms/ios/focus-input-via-button-no-scaling-expected.txt:
* LayoutTests/fast/forms/ios/focus-input-via-button-no-scaling.html:
* LayoutTests/fast/forms/ios/resources/zooming-test-utils.js:
* LayoutTests/fast/forms/ios/scroll-to-reveal-focused-select.html:
* LayoutTests/fast/forms/ios/user-scalable-does-not-scale-for-keyboard-focus-with-author-defined-scale-expected.txt:
* LayoutTests/fast/forms/ios/user-scalable-does-not-scale-for-keyboard-focus-with-author-defined-scale.html:
* LayoutTests/fast/forms/ios/user-scalable-does-not-scale-for-keyboard-focus-with-user-scalable-no-expected.txt:
* LayoutTests/fast/forms/ios/user-scalable-does-not-scale-for-keyboard-focus-with-user-scalable-no.html:
* LayoutTests/fast/shadow-dom/ios/accessory-bar-work-on-input-with-tabindex-in-shadow-tree.html:
* LayoutTests/platform/ios-wk2/TestExpectations:
* LayoutTests/platform/ios/fast/forms/ios/accessory-bar-navigation-expected.txt: Removed.
* LayoutTests/platform/ios/fast/forms/ios/focus-input-via-button-no-scaling-expected.txt: Removed.

See (3) above for more details; this also generally cleans up all of these affected layout tests,
which (even without any of these changes) are flaky or failing, and also have redundant
expectations, none of which have been updated to work with the iPhone 12 model that's now used for
our test runners.

This patch modernizes these tests by deploying new `UIHelper` methods, and also rebaselines and
reenables the failing tests.

* LayoutTests/resources/ui-helper.js:
(window.UIHelper.isZoomingOrScrolling):
(window.UIHelper.async waitForZoomingOrScrollingToEnd):

Add helper methods to (respectively) determine whether there are active zooming/scrolling animations
on the web view's scroll view, and also to wait for these animations to end.

(window.UIHelper.moveToPrevByKeyboardAccessoryBar): Deleted.

Rename `moveToPrevByKeyboardAccessoryBar` to `moveToPreviousByKeyboardAccessoryBar`, to be more
consistent with general WebKit code style.

* Source/WebKit/Platform/spi/ios/UIKitSPI.h:

Remove now-unused SPI declarations for `-_zoomToCenter:scale:duration:force:` and `-_zoomToCenter:scale:duration:`.

* Source/WebKit/UIProcess/API/ios/WKWebViewIOS.mm:
(-[WKWebView _zoomToCenter:atScale:animated:honorScrollability:]):

Implement the main fix — see (1) and (2) above. Note that we add an `honorScrollability` parameter here to preserve
existing behavior when double-tapping-to-scroll (i.e. the case where the user double-taps-to-zoom, but the zoom scale
doesn't actually change, so we just scroll instead). In the case where the root is `overflow: hidden;`, double-tapping
to scroll should have no effect; previously, this worked because we used the "non-force" version of `_zoomToCenter:`,
which respects scrollability. In contrast, we use `force:YES` for the keyboard scrolling codepath, and so we continue
using that there.

(-[WKWebView _zoomToRect:atScale:origin:animated:]):
(-[WKWebView _zoomOutWithOrigin:animated:]):
(-[WKWebView _zoomToInitialScaleWithOrigin:animated:]):
(-[WKWebView _zoomToFocusRect:selectionRect:fontSize:minimumScale:maximumScale:allowScaling:forceScroll:]):
(-[WKWebView _zoomToPoint:atScale:animated:]): Deleted.

Rename this from `_zoomToPoint:` to `_zoomToCenter:` for clarity, and also use this when scrolling
to reveal focused elements.

* Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
* Tools/TestRunnerShared/UIScriptContext/UIScriptController.h:
(WTR::UIScriptController::isZoomingOrScrolling const):
* Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.h:
* Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:
(-[TestRunnerWKWebView isZoomingOrScrolling]):
(-[TestRunnerWKWebView _stableStateOverride]):
(-[TestRunnerWKWebView _setStableStateOverride:]):

Drive-by style fix: replace `m_` with just an underscore (since this is a ivar defined in an
Objective-C class, rather than a C++ class member).

* Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h:
* Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptControllerIOS::isZoomingOrScrolling const):

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