Skip to content

Commit

Permalink
Move off of UIKit SPI: -[UIScrollView _zoomToCenter:scale:duration:fo…
Browse files Browse the repository at this point in the history
…rce:]

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
  • Loading branch information
whsieh committed Aug 23, 2023
1 parent a3419ec commit 83b571e
Show file tree
Hide file tree
Showing 24 changed files with 174 additions and 196 deletions.
13 changes: 6 additions & 7 deletions LayoutTests/fast/forms/ios/accessory-bar-navigation-expected.txt
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,13 @@ Tests zooming into a text input on tap.

After focus first field

tap location { x: 38.000, y: 446.000 }
scale 0.749
visibleRect { left: 0.000, top: 110.753, width: 427.000, height: 731.237 }
scale 0.750
visibleRect { left: 0, top: 0, width: 520, height: 1063 }
After move to second field

scale 0.749
visibleRect { left: 0.000, top: 932.061, width: 427.000, height: 731.237 }
scale 0.750
visibleRect { left: 0, top: 766, width: 520, height: 1063 }
After move to first field

scale 0.749
visibleRect { left: 0.000, top: 110.753, width: 427.000, height: 731.237 }
scale 0.750
visibleRect { left: 0, top: 0, width: 520, height: 1063 }
89 changes: 16 additions & 73 deletions LayoutTests/fast/forms/ios/accessory-bar-navigation.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,86 +17,29 @@
</style>

<script src="resources/zooming-test-utils.js"></script>
<script src="../../../resources/ui-helper.js"></script>
<script>
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
}

function getMoveUIScript(nextOrPrevious)
{
return `
(function() {
uiController.didEndZoomingCallback = function() {
var result = {
'scale' : uiController.zoomScale,
'visibleRect' : uiController.contentVisibleRect
};
var result = JSON.stringify(result, function(key, value) {
if (typeof value === "number")
return value.toFixed(3);
return value;
});
uiController.uiScriptComplete(result);
};
uiController.keyboardAccessoryBar${nextOrPrevious}();
})();`
}

function moveToNextField()
async function doTest()
{
var uiScript = getMoveUIScript('Next');
testRunner.runUIScript(uiScript, function(result) {
var header = document.createElement('h1');
header.textContent = 'After move to second field';
document.body.appendChild(header);

var results = tableFromJSON(result);
document.body.appendChild(results);

moveToPreviousField();
});
}
if (!window.testRunner || !testRunner.runUIScript)
return;

function moveToPreviousField()
{
var uiScript = getMoveUIScript('Previous');
testRunner.runUIScript(uiScript, function(result) {
var header = document.createElement('h1');
header.textContent = 'After move to first field';
document.body.appendChild(header);
testRunner.dumpAsText();
testRunner.waitUntilDone();

var results = tableFromJSON(result);
document.body.appendChild(results);
testRunner.notifyDone();
});
}

function tapOnElement(targetElement, xOffset, yOffset)
{
var point = getPointInsideElement(targetElement, xOffset, yOffset);
await UIHelper.activateElementAndWaitForInputSession(document.getElementById('input'));
await UIHelper.waitForZoomingOrScrollingToEnd();
await dumpScaleAndVisibleRect('After focus first field');

var uiScript = zoomAfterSingleTapUIScript(point.x, point.y);
testRunner.runUIScript(uiScript, function(result) {
var header = document.createElement('h1');
header.textContent = 'After focus first field';
document.body.appendChild(header);
await UIHelper.moveToNextByKeyboardAccessoryBar();
await UIHelper.waitForZoomingOrScrollingToEnd();
await dumpScaleAndVisibleRect('After move to second field');

var results = tableFromJSON(result);
document.body.appendChild(results);

moveToNextField();
});
}

function doTest()
{
if (!window.testRunner || !testRunner.runUIScript)
return;
await UIHelper.moveToPreviousByKeyboardAccessoryBar();
await UIHelper.waitForZoomingOrScrollingToEnd();
await dumpScaleAndVisibleRect('After move to first field');

tapOnElement(document.getElementById('input'), 10, 10);
testRunner.notifyDone();
}

window.addEventListener('load', doTest, false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,5 @@ Tests zooming into a text input on tap.

Click to focus input

tap location { x: 20.000, y: 62.000 }
scale 0.749
visibleRect { left: 0.000, top: 548.428, width: 427.000, height: 731.237 }
scale 0.750
visibleRect { left: 0, top: 381, width: 520, height: 1063 }
22 changes: 14 additions & 8 deletions LayoutTests/fast/forms/ios/focus-input-via-button-no-scaling.html
Original file line number Diff line number Diff line change
Expand Up @@ -11,22 +11,28 @@
display: block;
}
</style>

<script src="resources/zooming-test-utils.js"></script>
<script src="../../../resources/ui-helper.js"></script>
<script>
if (window.testRunner) {
testRunner.dumpAsText();
testRunner.waitUntilDone();
}

function buttonClicked()
{
document.getElementById('input').focus();
}

function doTest()
async function doTest()
{
testZoomAfterTap(document.getElementById('target'), 10, 10);
if (!window.testRunner || !testRunner.runUIScript)
return;

testRunner.dumpAsText();
testRunner.waitUntilDone();

await UIHelper.activateElementAndWaitForInputSession(document.getElementById('target'));
await UIHelper.waitForZoomingOrScrollingToEnd();
await dumpScaleAndVisibleRect();

testRunner.notifyDone();
}

window.addEventListener('load', doTest, false);
Expand Down
28 changes: 28 additions & 0 deletions LayoutTests/fast/forms/ios/resources/zooming-test-utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -74,3 +74,31 @@ function tableFromJSON(value)

return table;
}

async function dumpScaleAndVisibleRect(headerDescription)
{
const result = await new Promise((resolve) => {
testRunner.runUIScript(`(() => {
let result = {
scale : uiController.zoomScale,
visibleRect : uiController.contentVisibleRect
};
uiController.uiScriptComplete(JSON.stringify(result, (key, value) => {
if (typeof value !== "number")
return value;
if (key === "scale")
return value.toFixed(3);
return Math.round(value);
}));
})();`, resolve);
});

if (headerDescription !== undefined) {
const header = document.createElement('h1');
header.textContent = headerDescription;
document.body.appendChild(header);
}

const results = tableFromJSON(result);
document.body.appendChild(results);
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ Verifies that focusing a select element after user interaction scrolls to reveal
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".

PASS document.activeElement is document.getElementById('bottom')
PASS pageYOffset is >= 1000
PASS pageYOffset is >= 900
PASS successfullyParsed is true

TEST COMPLETE
Expand Down
18 changes: 3 additions & 15 deletions LayoutTests/fast/forms/ios/scroll-to-reveal-focused-select.html
Original file line number Diff line number Diff line change
Expand Up @@ -29,19 +29,6 @@
<script>
jsTestIsAsync = true;

async function tapAtLocationAndWaitForScrollingToEnd(x, y)
{
const uiScript = `
let doneCount = 0;
function checkDone() {
if (++doneCount == 2)
uiController.uiScriptComplete();
}
uiController.didEndZoomingCallback = checkDone;
uiController.singleTapAtPoint(${x}, ${y}, checkDone);`;
return new Promise(resolve => testRunner.runUIScript(uiScript, resolve));
}

async function run()
{
description("Verifies that focusing a select element after user interaction scrolls to reveal the select element. "
Expand All @@ -57,11 +44,12 @@
await UIHelper.waitForContextMenuToHide();

// Focus the second select element.
await tapAtLocationAndWaitForScrollingToEnd(160, 120);
await UIHelper.activateAt(160, 120);
await UIHelper.waitForContextMenuToShow();
await UIHelper.waitForZoomingOrScrollingToEnd();

shouldBe("document.activeElement", "document.getElementById('bottom')");
shouldBeGreaterThanOrEqual("pageYOffset", "1000");
shouldBeGreaterThanOrEqual("pageYOffset", "900");

document.activeElement.blur();
await UIHelper.waitForContextMenuToHide();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ This tests that even though force user scalable = true, we won't scale if a text
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


Zoom scale is: 1
PASS zoomScaleAfterFocusing is 1
PASS successfullyParsed is true

TEST COMPLETE
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
<html>
<head>
<script src="../../../resources/js-test-pre.js"></script>
<script src="../../../resources/js-test.js"></script>
<script src="../../../resources/ui-helper.js"></script>
<meta name="viewport" content="width=device-width, maximum-scale=1">
<script id="ui-script" type="text/plain">
(function() {
uiController.didEndZoomingCallback = function() {
uiController.uiScriptComplete(uiController.zoomScale);
};

uiController.singleTapAtPoint(10, 10, function() {});
})();
</script></head>
</head>

<body onload="runTest();">
<input type="text" id="textfield">
Expand All @@ -23,23 +16,21 @@
<script>
description("This tests that even though force user scalable = true, we won't scale if a text field gets focus and the page has maximum-scale=1");

if (window.testRunner) {
window.jsTestIsAsync = true;
jsTestIsAsync = true;
if (window.testRunner)
testRunner.setIgnoresViewportScaleLimits(true);
}

function runTest()
async function runTest()
{
if (testRunner.runUIScript) {
var uiScript = document.getElementById('ui-script').text;
testRunner.runUIScript(document.getElementById('ui-script').text, function(result) {
debug("Zoom scale is: " + result);
finishJSTest();
});
await UIHelper.activateAndWaitForInputSessionAt(10, 10);
await UIHelper.waitForZoomingOrScrollingToEnd();
zoomScaleAfterFocusing = await UIHelper.zoomScale();
shouldBe("zoomScaleAfterFocusing", "1");
finishJSTest();
}
}
</script>

<script src="../../../resources/js-test-post.js"></script>
</body>
</html>
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ This tests that even though force user scalable = true, we won't scale if a text
On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".


Zoom scale is: 1
PASS zoomScaleAfterFocusing is 1
PASS successfullyParsed is true

TEST COMPLETE
Expand Down
Original file line number Diff line number Diff line change
@@ -1,17 +1,10 @@
<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
<html>
<head>
<script src="../../../resources/js-test-pre.js"></script>
<script src="../../../resources/js-test.js"></script>
<script src="../../../resources/ui-helper.js"></script>
<meta name="viewport" content="width=device-width, user-scalable=no">
<script id="ui-script" type="text/plain">
(function() {
uiController.didEndZoomingCallback = function() {
uiController.uiScriptComplete(uiController.zoomScale);
};

uiController.singleTapAtPoint(10, 10, function() {});
})();
</script></head>
</head>

<body onload="runTest();">
<input type="text" id="textfield">
Expand All @@ -23,19 +16,18 @@
<script>
description("This tests that even though force user scalable = true, we won't scale if a text field gets focus and the page has user-scalable=no");

if (window.testRunner) {
window.jsTestIsAsync = true;
jsTestIsAsync = true;
if (window.testRunner)
testRunner.setIgnoresViewportScaleLimits(true);
}

function runTest()
async function runTest()
{
if (testRunner.runUIScript) {
var uiScript = document.getElementById('ui-script').text;
testRunner.runUIScript(document.getElementById('ui-script').text, function(result) {
debug("Zoom scale is: " + result);
finishJSTest();
});
await UIHelper.activateAndWaitForInputSessionAt(10, 10);
await UIHelper.waitForZoomingOrScrollingToEnd();
zoomScaleAfterFocusing = await UIHelper.zoomScale();
shouldBe("zoomScaleAfterFocusing", "1");
finishJSTest();
}
}
</script>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ <h1>Test that Accessory bar next/previous buttons work on inputs which is not as

for (let i = stack.length - 2; -1 < i; --i) {
debug(`back to ${String(i)}th input`);
await UIHelper.moveToPrevByKeyboardAccessoryBar();
await UIHelper.moveToPreviousByKeyboardAccessoryBar();
const { documentOrShadowRoot, inputElement, } = stack[i];
const activeElement = documentOrShadowRoot.activeElement;
debug(`documentOrShadowRoot.activeElement's placeholder attribute: ${activeElement.getAttribute('placeholder')}`);
Expand Down
1 change: 0 additions & 1 deletion LayoutTests/platform/ios-wk2/TestExpectations
Original file line number Diff line number Diff line change
Expand Up @@ -2079,7 +2079,6 @@ fast/events/ios/viewport-no-width-value-allows-double-tap.html [ Skip ]
fast/visual-viewport/ios/stable-update-with-keyboard.html [ Skip ]
media/modern-media-controls/media-documents/media-document-audio-ios-sizing.html [ Skip ]
media/modern-media-controls/media-documents/media-document-video-iphone-sizing.html [ Skip ]
fast/forms/ios/scroll-to-reveal-focused-select.html [ Failure ]
fast/scrolling/ios/body-overflow-hidden-height-100-percent-zoomed-1.html [ Failure ]

# This test "fails" on iPhone 12 simulator because it supports P3, may need to rebaseline this reftest
Expand Down

This file was deleted.

Loading

0 comments on commit 83b571e

Please sign in to comment.