When the iPhone keyboard is up, sometimes we never commit a stable update and re...
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 9 Dec 2017 00:00:09 +0000 (00:00 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 9 Dec 2017 00:00:09 +0000 (00:00 +0000)
https://bugs.webkit.org/show_bug.cgi?id=180498

Reviewed by Tim Horton.

Source/WebKit:

When the keyboard is showing, we would think that the page was in a rubber-banding state
because contentOffsetBoundedInValidRange() would always clamp the content offset to a different
value.

This happened because scrollView.contentInset don't change when the keyboard is showing,
but UIKit actually consults scrollView.adjustedContentInset, which does. If we use
scrollView.adjustedContentInset in this computation, we'll get a correct answer.

Also rewrote the clamping logic to be more similar to what UIKit does internally when computing
min/max content offset.

* UIProcess/API/Cocoa/WKWebView.mm:
(contentOffsetBoundedInValidRange):

LayoutTests:

Test that completes once a stable update is received after showing the keyboard.

* fast/visual-viewport/ios/stable-update-with-keyboard-expected.txt: Added.
* fast/visual-viewport/ios/stable-update-with-keyboard.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/visual-viewport/ios/stable-update-with-keyboard-expected.txt [new file with mode: 0644]
LayoutTests/fast/visual-viewport/ios/stable-update-with-keyboard.html [new file with mode: 0644]
Source/WebKit/ChangeLog
Source/WebKit/UIProcess/API/Cocoa/WKWebView.mm

index e3e2967..6bec546 100644 (file)
@@ -1,3 +1,15 @@
+2017-12-06  Simon Fraser  <simon.fraser@apple.com>
+
+        When the iPhone keyboard is up, sometimes we never commit a stable update and re-show the caret
+        https://bugs.webkit.org/show_bug.cgi?id=180498
+
+        Reviewed by Tim Horton.
+
+        Test that completes once a stable update is received after showing the keyboard.
+
+        * fast/visual-viewport/ios/stable-update-with-keyboard-expected.txt: Added.
+        * fast/visual-viewport/ios/stable-update-with-keyboard.html: Added.
+
 2017-12-08  Daniel Bates  <dabates@apple.com>
 
         Remove unnecessary prefix from AutoFillButtonType enumerators
diff --git a/LayoutTests/fast/visual-viewport/ios/stable-update-with-keyboard-expected.txt b/LayoutTests/fast/visual-viewport/ios/stable-update-with-keyboard-expected.txt
new file mode 100644 (file)
index 0000000..54f4d6e
--- /dev/null
@@ -0,0 +1,8 @@
+PASS: got stable update
+PASS successfullyParsed is true
+
+TEST COMPLETE
+This test will time out if no stable update is received.
+
+
+
diff --git a/LayoutTests/fast/visual-viewport/ios/stable-update-with-keyboard.html b/LayoutTests/fast/visual-viewport/ios/stable-update-with-keyboard.html
new file mode 100644 (file)
index 0000000..214bc2e
--- /dev/null
@@ -0,0 +1,71 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+
+<html>
+<head>
+    <meta name="viewport" content="initial-scale=1.0">
+    <style>
+        .spacer-above {
+            background-color: silver;
+            height: 600px;
+        }
+        .spacer-below {
+            background-color: silver;
+            height: 80px;
+        }
+        input {
+            display: block;
+            margin: 60px 30px;
+        }
+    </style>
+    <script src="../../../resources/js-test-pre.js"></script>
+    <script>
+        var jsTestIsAsync = true;
+
+        function getScrollDownUIScript()
+        {
+            return `(function() {
+                uiController.immediateScrollToOffset(0, 800);
+            })();`;
+        }
+        
+        function getFocusInputUIScript(x, y)
+        {
+            return `(function() {
+
+                uiController.didShowKeyboardCallback = function() {
+                    uiController.doAfterNextStablePresentationUpdate(function() {
+                        uiController.uiScriptComplete();
+                    });
+                };
+                uiController.singleTapAtPoint(${x}, ${y}, function() {});
+            })();`;
+        }
+
+        function runTest()
+        {
+            if (!window.testRunner || !testRunner.runUIScript)
+                return;
+            
+            testRunner.runUIScript(getScrollDownUIScript(), function() {
+                window.setTimeout(function() {
+                    var rect = document.getElementById('input').getBoundingClientRect();
+                    // singleTapAtPoint takes document coordinates, so add scrollTop to clientRect.top.
+                    testRunner.runUIScript(getFocusInputUIScript(rect.left, rect.top + document.scrollingElement.scrollTop), function() {
+                        debug('PASS: got stable update');
+                        finishJSTest();
+                    });
+                }, 0);
+            });
+        }
+
+        window.addEventListener('load', runTest, false);
+    </script>
+</head>
+<body>
+<p>This test will time out if no stable update is received.</p>
+<div class="spacer-above"></div>
+<input id="input" type="text"/>
+<div class="spacer-below"></div>
+<script src="../../../resources/js-test-post.js"></script>
+</body>
+</html>
index 2d5fafa..7318411 100644 (file)
@@ -1,3 +1,24 @@
+2017-12-06  Simon Fraser  <simon.fraser@apple.com>
+
+        When the iPhone keyboard is up, sometimes we never commit a stable update and re-show the caret
+        https://bugs.webkit.org/show_bug.cgi?id=180498
+
+        Reviewed by Tim Horton.
+
+        When the keyboard is showing, we would think that the page was in a rubber-banding state
+        because contentOffsetBoundedInValidRange() would always clamp the content offset to a different
+        value.
+
+        This happened because scrollView.contentInset don't change when the keyboard is showing,
+        but UIKit actually consults scrollView.adjustedContentInset, which does. If we use
+        scrollView.adjustedContentInset in this computation, we'll get a correct answer.
+
+        Also rewrote the clamping logic to be more similar to what UIKit does internally when computing
+        min/max content offset.
+
+        * UIProcess/API/Cocoa/WKWebView.mm:
+        (contentOffsetBoundedInValidRange):
+
 2017-12-08  Chris Dumez  <cdumez@apple.com>
 
         Clearing all Website Data should remove service worker registrations on disk
index 30afed8..0688a6e 100644 (file)
@@ -1629,18 +1629,19 @@ static WebCore::Color scrollViewBackgroundColor(WKWebView *webView)
 
 static CGPoint contentOffsetBoundedInValidRange(UIScrollView *scrollView, CGPoint contentOffset)
 {
+#if __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000
+    UIEdgeInsets contentInsets = scrollView.adjustedContentInset;
+#else
     UIEdgeInsets contentInsets = scrollView.contentInset;
+#endif
+
     CGSize contentSize = scrollView.contentSize;
     CGSize scrollViewSize = scrollView.bounds.size;
 
-    CGFloat maxHorizontalOffset = contentSize.width + contentInsets.right - scrollViewSize.width;
-    contentOffset.x = std::min(maxHorizontalOffset, contentOffset.x);
-    contentOffset.x = std::max(-contentInsets.left, contentOffset.x);
+    CGPoint minimumContentOffset = CGPointMake(-contentInsets.left, -contentInsets.top);
+    CGPoint maximumContentOffset = CGPointMake(std::max(minimumContentOffset.x, contentSize.width + contentInsets.right - scrollViewSize.width), std::max(minimumContentOffset.y, contentSize.height + contentInsets.bottom - scrollViewSize.height));
 
-    CGFloat maxVerticalOffset = contentSize.height + contentInsets.bottom - scrollViewSize.height;
-    contentOffset.y = std::min(maxVerticalOffset, contentOffset.y);
-    contentOffset.y = std::max(-contentInsets.top, contentOffset.y);
-    return contentOffset;
+    return CGPointMake(std::max(std::min(contentOffset.x, maximumContentOffset.x), minimumContentOffset.x), std::max(std::min(contentOffset.y, maximumContentOffset.y), minimumContentOffset.y));
 }
 
 static void changeContentOffsetBoundedInValidRange(UIScrollView *scrollView, WebCore::FloatPoint contentOffset)