Attachment 371045 Details for Bug 198412 – Fixes the bug

[patch] Fixes the bug

bug-198412-20190531015314.patch (text/plain), 8.54 KB, created by Ryosuke Niwa on 2019-05-31 01:53:14 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Ryosuke Niwa

Created: 2019-05-31 01:53:14 PDT

Size: 8.54 KB

patch

obsolete

>Subversion Revision: 245847
>diff --git a/Source/WebKit/ChangeLog b/Source/WebKit/ChangeLog
>index 2345a6eaf7d40a4ae529f4b3611bef2f8236e98a..478bfbbd24cc01b28ceb24483dc75c10580005b4 100644
>--- a/Source/WebKit/ChangeLog
>+++ b/Source/WebKit/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        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-30  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         iOS: Main frame should be scrollable when pinch zoomed or software keyboard is up
>diff --git a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>index fa3f1e2db5f65ac4ac81e2235958f7d53427439d..fe8ed889272a2deb5438cc5a1ea00bb29899b41f 100644
>--- a/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>+++ b/Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm
>@@ -2327,13 +2327,15 @@ - (void)_zoomToFocusRect:(const WebCore::FloatRect&)focusedElementRectInDocument
>     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 @@ - (void)_zoomToFocusRect:(const WebCore::FloatRect&)focusedElementRectInDocument
>         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()]))
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index ccfb420b82bdae40374d0a451bbf4c229f0a9a5d..10654ae10b0b3329e7cf0d83f58fb38d2390ff09 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -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 NOBODY (OOPS!).
>+
>+        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-30  Ryosuke Niwa  <rniwa@webkit.org>
> 
>         iOS: Main frame should be scrollable when pinch zoomed or software keyboard is up
>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
>index 0000000000000000000000000000000000000000..596f157c1f1a15b978c011548571a9827c986eb2
>--- /dev/null
>+++ b/LayoutTests/fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad-expected.txt
>@@ -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
>index 0000000000000000000000000000000000000000..63da36fa3e95ce47edcecdaa685ca21dce2d6406
>--- /dev/null
>+++ b/LayoutTests/fast/scrolling/ios/reveal-focused-element-right-above-keyboard-on-ipad.html
>@@ -0,0 +1,72 @@
>+<!DOCTYPE html> 
>+<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>

Actions: View | Formatted Diff | Diff

Attachments on bug 198412: 371045