Title: [245993] trunk
Revision
245993
Author
rn...@webkit.org
Date
2019-05-31 16:13:39 -0700 (Fri, 31 May 2019)

Log Message

[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.

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.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (245992 => 245993)


--- trunk/LayoutTests/ChangeLog	2019-05-31 23:10:27 UTC (rev 245992)
+++ trunk/LayoutTests/ChangeLog	2019-05-31 23:13:39 UTC (rev 245993)
@@ -1,3 +1,16 @@
+2019-05-31  Ryosuke Niwa  <rn...@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  <nvasil...@apple.com>
 
         Web Inspector: CSS Changes: modifications aren't shared for rules that match multiple elements

Added: trunk/LayoutTests/fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad-expected.txt (0 => 245993)


--- trunk/LayoutTests/fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad-expected.txt	2019-05-31 23:13:39 UTC (rev 245993)
@@ -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
+

Added: trunk/LayoutTests/fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad.html (0 => 245993)


--- trunk/LayoutTests/fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad.html	                        (rev 0)
+++ trunk/LayoutTests/fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad.html	2019-05-31 23:13:39 UTC (rev 245993)
@@ -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=""
+<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>

Modified: trunk/Source/WebKit/ChangeLog (245992 => 245993)


--- trunk/Source/WebKit/ChangeLog	2019-05-31 23:10:27 UTC (rev 245992)
+++ trunk/Source/WebKit/ChangeLog	2019-05-31 23:13:39 UTC (rev 245993)
@@ -1,3 +1,20 @@
+2019-05-31  Ryosuke Niwa  <rn...@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  <cdu...@apple.com>
 
         Unreviewed, rolling out r245899.

Modified: trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm (245992 => 245993)


--- trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2019-05-31 23:10:27 UTC (rev 245992)
+++ trunk/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm	2019-05-31 23:13:39 UTC (rev 245993)
@@ -2327,13 +2327,15 @@
     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 @@
         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()]))
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to