[webkit-changes] [WebKit/WebKit] 184aa4: [iOS] [scroll-anchoring] Software keyboard overlap...

Wenson Hsieh noreply at github.com
Thu Jan 11 21:13:22 PST 2024


  Branch: refs/heads/main
  Home:   https://github.com/WebKit/WebKit
  Commit: 184aa40be65f3f0c946f78c44009078fc492cb2c
      https://github.com/WebKit/WebKit/commit/184aa40be65f3f0c946f78c44009078fc492cb2c
  Author: Wenson Hsieh <wenson_hsieh at apple.com>
  Date:   2024-01-11 (Thu, 11 Jan 2024)

  Changed paths:
    M LayoutTests/fast/scrolling/ios/scroll-anchoring-momentum-scroll.html
    A LayoutTests/fast/scrolling/ios/scroll-anchoring-when-revealing-focused-element-expected.txt
    A LayoutTests/fast/scrolling/ios/scroll-anchoring-when-revealing-focused-element.html
    M LayoutTests/resources/ui-helper.js
    M Source/WebCore/page/LocalFrameView.cpp
    M Source/WebKit/UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h
    M Source/WebKit/UIProcess/API/ios/WKWebViewTestingIOS.mm
    M Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl
    M Tools/TestRunnerShared/UIScriptContext/UIScriptController.h
    M Tools/WebKitTestRunner/ios/TestControllerIOS.mm
    M Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h
    M Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm

  Log Message:
  -----------
  [iOS] [scroll-anchoring] Software keyboard overlaps input field on webauthn.io
https://bugs.webkit.org/show_bug.cgi?id=267440
rdar://120753568

Reviewed by Simon Fraser.

It's possible for scroll anchoring to cause the scroll position to jump to the previously anchored
element, when scrolling to reveal the focused element on top of the software keyboard on iOS. This
scrolling (driven by `-_zoomToCenter:atScale:animated:` in the UI process) is bookended in Safari by
calls to `-_(begin|end)InteractiveObscuredInsetsChange` while the keyboard is showing, which puts us
in unstable state and also passes `ViewportRectStability::ChangingObscuredInsetsInteractively` into
`AsyncScrollingCoordinator::reconcileScrollingState`. This, in turn, means we'll skip setting the
layout viewport override rect when syncing the scroll position.

This isn't normally an issue, because we'll eventually update the layout viewport once we end
interactive obscured inset changes. However, in the context of scroll anchoring, the fact that the
scroll position is updated but the layout viewport rect isn't (when reconciling scrolling state)
means that it's possible for the scroll anchoring controller to adjust the layout viewport rect in
the following codepath during layout:

```
void LocalFrameView::updateLayoutViewport()
{
    …
    if (m_layoutViewportOverrideRect) {
        if (currentScrollType() == ScrollType::Programmatic) {
            LOG_WITH_STREAM(Scrolling, stream << "computing new override layout viewport because of programmatic scrolling");
            LayoutPoint newOrigin = computeLayoutViewportOrigin(visualViewportRect(), minStableLayoutViewportOrigin(), maxStableLayoutViewportOrigin(), layoutViewport, StickToDocumentBounds);
            setLayoutViewportOverrideRect(LayoutRect(newOrigin, m_layoutViewportOverrideRect.value().size()));
        }
        layoutOrVisualViewportChanged();
        return;
    }
```

..._without_ having properly invalidated and updated the current scroll anchor. In turn, this makes
it possible for us to compare the layout viewport after accounting for the keyboard scrolling amount
against a stale `m_lastOffsetForAnchorElement`, causing the adjustment to overcompensate and jump
back up to the top of the page in the middle of the keyboard scrolling animation.

To fix this, we simply invalidate the anchor element on `ScrollAnchoringController` (along with the
stale `m_lastOffsetForAnchorElement`) when the layout viewport changes.

* LayoutTests/fast/scrolling/ios/scroll-anchoring-momentum-scroll.html:

Drive-by fix: indentation.

* LayoutTests/fast/scrolling/ios/scroll-anchoring-when-revealing-focused-element-expected.txt: Added.
* LayoutTests/fast/scrolling/ios/scroll-anchoring-when-revealing-focused-element.html: Added.

Add a new layout test that fails without this fix. While it doesn't precisely simulate the bug as it
appears on webauthn.io, it exercises the same root cause by repeatedly triggering scroll anchoring
adjustment while scrolling to reveal the focused element with the software keyboard, all in the
scope of interactive obscured inset changes.

* LayoutTests/resources/ui-helper.js:
(window.UIHelper.beginInteractiveObscuredInsetsChange):
(window.UIHelper.endInteractiveObscuredInsetsChange):
(window.UIHelper.setObscuredInsets):

Add new `UIHelper` methods to allow layout tests to adjust the obscured insets in the same way that
Safari does when showing the software keyboard.

* Source/WebCore/page/LocalFrameView.cpp:
(WebCore::LocalFrameView::setLayoutViewportOverrideRect):

See comments above for more detail.

* Source/WebKit/UIProcess/API/ios/WKWebViewPrivateForTestingIOS.h:
* Source/WebKit/UIProcess/API/ios/WKWebViewTestingIOS.mm:
(-[WKWebView _resetObscuredInsetsForTesting]):

Add a testing hook to reset the web view's obscured insets; used when resetting state in between
layout tests.

* Tools/TestRunnerShared/UIScriptContext/Bindings/UIScriptController.idl:
* Tools/TestRunnerShared/UIScriptContext/UIScriptController.h:
(WTR::UIScriptController::beginInteractiveObscuredInsetsChange):
(WTR::UIScriptController::endInteractiveObscuredInsetsChange):
(WTR::UIScriptController::setObscuredInsets):

Add boilerplate code to implement the new script controller helper methods.

* Tools/WebKitTestRunner/ios/TestControllerIOS.mm:
(WTR::TestController::platformResetStateToConsistentValues):
* Tools/WebKitTestRunner/ios/UIScriptControllerIOS.h:
* Tools/WebKitTestRunner/ios/UIScriptControllerIOS.mm:
(WTR::UIScriptControllerIOS::beginInteractiveObscuredInsetsChange):
(WTR::UIScriptControllerIOS::setObscuredInsets):
(WTR::UIScriptControllerIOS::endInteractiveObscuredInsetsChange):

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




More information about the webkit-changes mailing list