[iOS] REGRESSION(r244851): Revealing caret sometimes fails when content inset is...
authorrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Jul 2019 21:12:47 +0000 (21:12 +0000)
committerrniwa@webkit.org <rniwa@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 10 Jul 2019 21:12:47 +0000 (21:12 +0000)
https://bugs.webkit.org/show_bug.cgi?id=199662

Reviewed by Simon Fraser.

Source/WebCore:

The bug was caused by ScrollableArea::maximumScrollPosition using ScrollableArea::visibleSize, which does not
take the content insets into account correctly, rather than FrameView::visualViewportRectExpandedByContentInsets,
which does, and is used for "viewRect" in RenderLayer::scrollRectToVisible.

Override the maximum scroll position using visualViewportRectExpandedByContentInsets in iOS to fix the issue.

Test: editing/selection/ios/autoscroll-with-top-content-inset-2.html

* rendering/RenderLayer.cpp:
(WebCore::RenderLayer::scrollRectToVisible): Fixed the bug.

LayoutTests:

Added a regression test.

* editing/selection/ios/autoscroll-with-top-content-inset-2-expected.txt: Added.
* editing/selection/ios/autoscroll-with-top-content-inset-2.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset-2-expected.txt [new file with mode: 0644]
LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset-2.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayer.cpp

index e2e072b..f7705ec 100644 (file)
@@ -1,3 +1,15 @@
+2019-07-10  Ryosuke Niwa  <rniwa@webkit.org>
+
+        [iOS] REGRESSION(r244851): Revealing caret sometimes fails when content inset is used
+        https://bugs.webkit.org/show_bug.cgi?id=199662
+
+        Reviewed by Simon Fraser.
+
+        Added a regression test.
+
+        * editing/selection/ios/autoscroll-with-top-content-inset-2-expected.txt: Added.
+        * editing/selection/ios/autoscroll-with-top-content-inset-2.html: Added.
+
 2019-07-10  Saam Barati  <sbarati@apple.com>
 
         [WHLSL Import more JS reference spec tests
diff --git a/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset-2-expected.txt b/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset-2-expected.txt
new file mode 100644 (file)
index 0000000..66b24d0
--- /dev/null
@@ -0,0 +1 @@
+PASS - caret appeared within the visual viewport
diff --git a/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset-2.html b/LayoutTests/editing/selection/ios/autoscroll-with-top-content-inset-2.html
new file mode 100644 (file)
index 0000000..210bd5f
--- /dev/null
@@ -0,0 +1,41 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true, contentInset.top=150 ] -->
+<html>
+<meta name="viewport" content="width=device-width, initial-scale=1">
+<head>
+<script src="../../../resources/basic-gestures.js"></script>
+<script src="../../../resources/ui-helper.js"></script>
+<script>
+
+if (window.testRunner) {
+    testRunner.waitUntilDone();
+    testRunner.dumpAsText();
+}
+
+async function run()
+{
+    await UIHelper.setHardwareKeyboardAttached(false);
+    await UIHelper.activateAndWaitForInputSessionAt(10, 10);
+
+    for (let i = 0; i < 150; i++)
+        document.execCommand('insertParagraph');
+
+    window.scrollTo(0, 0);
+    await UIHelper.ensurePresentationUpdate();
+    await UIHelper.enterText('hello');
+
+    const keyboardRect = await UIHelper.inputViewBounds();
+    const scrollTop = document.scrollingElement.scrollTop;
+    const viewportHeight = visualViewport.height;
+    const caretRect = getSelection().getRangeAt(0).getBoundingClientRect();
+
+    document.body.textContent = caretRect.y + caretRect.height <= visualViewport.height
+        ? 'PASS - caret appeared within the visual viewport'
+        : 'FAIL - caret appeared below the visual viewport';
+
+    if (window.testRunner)
+        testRunner.notifyDone();
+}
+</script>
+</head>
+<body contenteditable onload="run()"></body>
+</html>
index b76fdd5..f8c6b30 100644 (file)
@@ -1,3 +1,21 @@
+2019-07-10  Ryosuke Niwa  <rniwa@webkit.org>
+
+        [iOS] REGRESSION(r244851): Revealing caret sometimes fails when content inset is used
+        https://bugs.webkit.org/show_bug.cgi?id=199662
+
+        Reviewed by Simon Fraser.
+
+        The bug was caused by ScrollableArea::maximumScrollPosition using ScrollableArea::visibleSize, which does not
+        take the content insets into account correctly, rather than FrameView::visualViewportRectExpandedByContentInsets,
+        which does, and is used for "viewRect" in RenderLayer::scrollRectToVisible.
+
+        Override the maximum scroll position using visualViewportRectExpandedByContentInsets in iOS to fix the issue.
+
+        Test: editing/selection/ios/autoscroll-with-top-content-inset-2.html
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::RenderLayer::scrollRectToVisible): Fixed the bug.
+
 2019-07-10  Ryan Haddad  <ryanhaddad@apple.com>
 
         Unreviewed, rolling out r247316.
index af08396..8f11f46 100644 (file)
@@ -2678,10 +2678,13 @@ void RenderLayer::scrollRectToVisible(const LayoutRect& absoluteRect, bool insid
 #if !PLATFORM(IOS_FAMILY)
             LayoutRect viewRect = frameView.visibleContentRect();
 #else
-            // FIXME: ContentInsets should be taken care of in UI process side.
+            // FIXME: ContentInsets should be taken care of in UI process side. webkit.org/b/199682
             // To do that, getRectToExpose needs to return the additional scrolling to do beyond content rect.
             LayoutRect viewRect = frameView.visualViewportRectExpandedByContentInsets();
 
+            // FIXME: webkit.org/b/199683 FrameView::visibleContentRect is wrong when content insets are present
+            maxScrollPosition = frameView.scrollPositionFromOffset(ScrollPosition(frameView.totalContentsSize() - flooredIntSize(viewRect.size())));
+
             auto contentInsets = page().contentInsets();
             minScrollPosition.move(-contentInsets.left(), -contentInsets.top());
             maxScrollPosition.move(contentInsets.right(), contentInsets.bottom());