[iOS] Reveal the focused element when it's immediately above software keyboard
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 31 May 2019 23:13:39 +0000 (23:13 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 31 May 2019 23:13:39 +0000 (23:13 +0000)
https://bugs.webkit.org/show_bug.cgi?id=198412

Reviewed by Wenson Hsieh.

Source/WebKit:

When _zoomToRevealFocusedElement is called with forceScroll set to NO (happens when input type is none or drawing
or when the platform is iPad), we don't force scrolling to reveal the focused element when it's entirely visible.

This can be misleading in cases where there is more content right beneath it relevant for editing operations.
Zoom & scroll to reveal the focused element when the said element is within 50px of the software keyboard.

* Platform/spi/ios/UIKitSPI.h:
* UIProcess/API/Cocoa/WKWebView.mm:
(-[WKWebView _zoomToFocusRect:selectionRect:insideFixed:fontSize:minimumScale:maximumScale:allowScaling:forceScroll:]):

LayoutTests:

Added a regression test. Note that this test always passes on non-iPad platforms either
before or after this patch as _zoomToRevealFocusedElement forces scrolling in that case.

* fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad-expected.txt: Added.
* fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245993 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad-expected.txt [new file with mode: 0644]
LayoutTests/fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad.html [new file with mode: 0644]
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm

index f7103d5..4021d3d 100644 (file)
@@ -1,3 +1,16 @@
+2019-05-31  Ryosuke Niwa  <rniwa@webkit.org>
+
+        [iOS] Reveal the focused element when it's immediately above software keyboard
+        https://bugs.webkit.org/show_bug.cgi?id=198412
+
+        Reviewed by Wenson Hsieh.
+
+        Added a regression test. Note that this test always passes on non-iPad platforms either
+        before or after this patch as _zoomToRevealFocusedElement forces scrolling in that case.
+
+        * fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad-expected.txt: Added.
+        * fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad.html: Added.
+
 2019-05-31  Nikita Vasilyev  <nvasilyev@apple.com>
 
         Web Inspector: CSS Changes: modifications aren't shared for rules that match multiple elements
diff --git a/LayoutTests/fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad-expected.txt b/LayoutTests/fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad-expected.txt
new file mode 100644 (file)
index 0000000..596f157
--- /dev/null
@@ -0,0 +1,5 @@
+This tests focusing an element right above the keyboard. WebKit should scroll the document to reveal the element.
+To manually test, focus the text field below on iPad to bring up the docked software keyboard.
+Dimiss it and focus it again after the text field had moved. The document should scroll.
+PASS - the document did scroll
+
diff --git a/LayoutTests/fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad.html b/LayoutTests/fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad.html
new file mode 100644 (file)
index 0000000..63da36f
--- /dev/null
@@ -0,0 +1,72 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true internal:AsyncOverflowScrollingEnabled=true internal:AsyncFrameScrollingEnabled=true ] -->
+<html>
+<head>
+<meta name="viewport" content="width=device-width, initial-scale=1, user-scalable=no">
+<script src="../../../resources/ui-helper.js"></script>
+<style>
+html, body { width: 100%; height: 100%; margin: 0px; padding: 0px; }
+#content { width: 100%; height: 100%; box-sizing: border-box; padding: 20px; background: #ccc; }
+#target { position: absolute; }
+</style>
+<script>
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+}
+
+function listenForEventOnce(target, name, timeout) {
+    return new Promise((resolve, reject) => {
+        const timer = timeout ? setTimeout(reject, timeout) : null;
+        target.addEventListener(name, () => {
+            if (timer)
+                clearTimeout(timer);
+            resolve();
+        }, {once: true});
+    });
+}
+
+async function runTest() {
+    const target = document.getElementById('target');
+    const resizeEvent = listenForEventOnce(target, 'focus').then(() => listenForEventOnce(visualViewport, 'resize'));
+
+    if (window.testRunner) {
+        UIHelper.isIOS = () => true;
+        await UIHelper.setHardwareKeyboardAttached(false);
+        await UIHelper.activateElementAndWaitForInputSession(target);
+    }
+
+    await resizeEvent;
+
+    const keyboardHeight = document.documentElement.clientHeight - visualViewport.height;
+    target.style.bottom = (keyboardHeight + 20) + 'px';
+
+    if (window.testRunner) {
+        document.activeElement.blur();
+        await UIHelper.waitForKeyboardToHide();
+    }
+
+    const scrollEvent = listenForEventOnce(target, 'focus').then(() => listenForEventOnce(visualViewport, 'scroll', window.testRunner ? 3000 : 500));
+
+    if (window.testRunner)
+        await UIHelper.activateElementAndWaitForInputSession(target);
+
+    try {
+        await scrollEvent;
+    } catch (error) { }
+
+    document.getElementById('result').textContent = document.documentElement.scrollTop >= 50 ? 'PASS - the document did scroll' : 'FAIL - the document did not scroll';
+
+    if (window.testRunner)
+        testRunner.notifyDone();
+}
+
+</script>
+<body onload="runTest()">
+<div id="content">
+This tests focusing an element right above the keyboard. WebKit should scroll the document to reveal the element.<br>
+To manually test, focus the text field below on iPad to bring up the docked software keyboard.<br>
+Dimiss it and focus it again after the text field had moved. The document should scroll.<br>
+<div id="result"></div>
+<input id="target">
+</div>
+</html>
index 7c42b5c..21d977d 100644 (file)
@@ -1,3 +1,20 @@
+2019-05-31  Ryosuke Niwa  <rniwa@webkit.org>
+
+        [iOS] Reveal the focused element when it's immediately above software keyboard
+        https://bugs.webkit.org/show_bug.cgi?id=198412
+
+        Reviewed by Wenson Hsieh.
+
+        When _zoomToRevealFocusedElement is called with forceScroll set to NO (happens when input type is none or drawing
+        or when the platform is iPad), we don't force scrolling to reveal the focused element when it's entirely visible.
+
+        This can be misleading in cases where there is more content right beneath it relevant for editing operations.
+        Zoom & scroll to reveal the focused element when the said element is within 50px of the software keyboard.
+
+        * Platform/spi/ios/UIKitSPI.h:
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (-[WKWebView _zoomToFocusRect:selectionRect:insideFixed:fontSize:minimumScale:maximumScale:allowScaling:forceScroll:]):
+
 2019-05-31  Chris Dumez  <cdumez@apple.com>
 
         Unreviewed, rolling out r245899.
index fa3f1e2..fe8ed88 100644 (file)
@@ -2327,13 +2327,15 @@ static WebCore::FloatPoint constrainContentOffset(WebCore::FloatPoint contentOff
     CGSize visibleSize = visibleScrollViewBoundsInWebViewCoordinates.size;
 
     CGFloat visibleOffsetFromTop = 0;
+    CGFloat minimumDistanceFromKeyboardToTriggerScroll = 0;
     if (!CGRectIsEmpty(intersectionBetweenScrollViewAndFormAssistant)) {
         CGFloat heightVisibleAboveFormAssistant = CGRectGetMinY(intersectionBetweenScrollViewAndFormAssistant) - CGRectGetMinY(visibleScrollViewBoundsInWebViewCoordinates);
         CGFloat heightVisibleBelowFormAssistant = CGRectGetMaxY(visibleScrollViewBoundsInWebViewCoordinates) - CGRectGetMaxY(intersectionBetweenScrollViewAndFormAssistant);
 
-        if (heightVisibleAboveFormAssistant >= minimumHeightToShowContentAboveKeyboard || heightVisibleBelowFormAssistant < heightVisibleAboveFormAssistant)
+        if (heightVisibleAboveFormAssistant >= minimumHeightToShowContentAboveKeyboard || heightVisibleBelowFormAssistant < heightVisibleAboveFormAssistant) {
             visibleSize.height = heightVisibleAboveFormAssistant;
-        else {
+            minimumDistanceFromKeyboardToTriggerScroll = 50;
+        } else {
             visibleSize.height = heightVisibleBelowFormAssistant;
             visibleOffsetFromTop = CGRectGetMaxY(intersectionBetweenScrollViewAndFormAssistant) - CGRectGetMinY(visibleScrollViewBoundsInWebViewCoordinates);
         }
@@ -2372,6 +2374,7 @@ static WebCore::FloatPoint constrainContentOffset(WebCore::FloatPoint contentOff
         currentlyVisibleRegionInWebViewCoordinates.origin = unobscuredScrollViewRectInWebViewCoordinates.origin;
         currentlyVisibleRegionInWebViewCoordinates.origin.y += visibleOffsetFromTop;
         currentlyVisibleRegionInWebViewCoordinates.size = visibleSize;
+        currentlyVisibleRegionInWebViewCoordinates.size.height -= minimumDistanceFromKeyboardToTriggerScroll;
 
         // Don't bother scrolling if the entire node is already visible, whether or not we got a selectionRect.
         if (CGRectContainsRect(currentlyVisibleRegionInWebViewCoordinates, [self convertRect:focusedElementRectInDocumentCoordinates fromView:_contentView.get()]))