Title: [227430] trunk
Revision
227430
Author
simon.fra...@apple.com
Date
2018-01-23 12:11:10 -0800 (Tue, 23 Jan 2018)

Log Message

Element with position:fixed stops scrolling at the bottom of the page, but is painted in the right place on Chacos.com.
https://bugs.webkit.org/show_bug.cgi?id=181741
rdar://problem/36593581

Reviewed by Tim Horton.
Source/WebCore:

The #ifdef for iOS was wrong; on iOS, visibleSize() is in content coordinates and matches
unscaledDocumentRect, so there's no need to scale it. Doing so computed the wrong unscaledMaximumScrollPosition
which broke hit-testing when the document minimum scale was > 1.

Test: fast/visual-viewport/ios/min-scale-greater-than-one.html

* page/FrameView.cpp:
(WebCore::FrameView::unscaledMaximumScrollPosition const):

Tools:

If zoomToScale:animated: is called with the current zoom level, call the callback
rather than doing nothing.

* WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:
(-[TestRunnerWKWebView zoomToScale:animated:completionHandler:]):

LayoutTests:

* fast/visual-viewport/ios/min-scale-greater-than-one-expected.txt: Added.
* fast/visual-viewport/ios/min-scale-greater-than-one.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (227429 => 227430)


--- trunk/LayoutTests/ChangeLog	2018-01-23 20:08:36 UTC (rev 227429)
+++ trunk/LayoutTests/ChangeLog	2018-01-23 20:11:10 UTC (rev 227430)
@@ -1,3 +1,14 @@
+2018-01-23  Simon Fraser  <simon.fra...@apple.com>
+
+        Element with position:fixed stops scrolling at the bottom of the page, but is painted in the right place on Chacos.com.
+        https://bugs.webkit.org/show_bug.cgi?id=181741
+        rdar://problem/36593581
+
+        Reviewed by Tim Horton.
+
+        * fast/visual-viewport/ios/min-scale-greater-than-one-expected.txt: Added.
+        * fast/visual-viewport/ios/min-scale-greater-than-one.html: Added.
+
 2018-01-23  Antoine Quint  <grao...@apple.com>
 
         [Web Animations] Expose getKeyframes() and parsing of remaining keyframe properties

Added: trunk/LayoutTests/fast/visual-viewport/ios/min-scale-greater-than-one-expected.txt (0 => 227430)


--- trunk/LayoutTests/fast/visual-viewport/ios/min-scale-greater-than-one-expected.txt	                        (rev 0)
+++ trunk/LayoutTests/fast/visual-viewport/ios/min-scale-greater-than-one-expected.txt	2018-01-23 20:11:10 UTC (rev 227430)
@@ -0,0 +1,10 @@
+PASS Math.round(boundingClientRect.top) is expectedTop
+Scrolling to bottom
+PASS window.scrollY is expectedTop
+PASS Math.round(boundingClientRect.top) is expectedTop
+Tap dispatched
+Target clicked
+PASS successfullyParsed is true
+
+TEST COMPLETE
+

Added: trunk/LayoutTests/fast/visual-viewport/ios/min-scale-greater-than-one.html (0 => 227430)


--- trunk/LayoutTests/fast/visual-viewport/ios/min-scale-greater-than-one.html	                        (rev 0)
+++ trunk/LayoutTests/fast/visual-viewport/ios/min-scale-greater-than-one.html	2018-01-23 20:11:10 UTC (rev 227430)
@@ -0,0 +1,83 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ useFlexibleViewport=true ] -->
+
+<html>
+<head>
+    <meta name="viewport" content="width=256, initial-scale=1.25">
+    <script src=""
+    <script src=""
+    <script>
+        jsTestIsAsync = true;
+
+        function scrollToBottom(offsetTop)
+        {
+            return `(function() {
+                uiController.immediateScrollToOffset(0, ${offsetTop});
+                uiController.doAfterPresentationUpdate(function() {
+                    uiController.uiScriptComplete();
+                });
+            })()`;
+        }
+
+        var boundingClientRect;
+        var expectedTop;
+        function checkBoundingClientRect(top)
+        {
+            var target = document.getElementById('target');
+            boundingClientRect = target.getBoundingClientRect();
+            expectedTop = top;
+            shouldBe('Math.round(boundingClientRect.top)', 'expectedTop');
+        }
+        
+        function targetClicked()
+        {
+            debug('Target clicked');
+            finishJSTest();
+        }
+
+        function doTest()
+        {
+            var scale = 320 / 250;
+
+            checkBoundingClientRect(300);
+            debug('Scrolling to bottom');
+            testRunner.runUIScript(scrollToBottom(2000), function() {
+
+                expectedTop = 1578;
+                shouldBe('window.scrollY', 'expectedTop');
+                checkBoundingClientRect(300);
+                
+                // Tap coordinates are document coordinates.
+                var top = window.scrollY + 300 + 10;
+                UIHelper.tapAt(50, top).then(() => {
+                    debug('Tap dispatched');
+                    setTimeout(function watchdog() {
+                        debug('Failed to receive click');
+                        finishJSTest();
+                    }, 400);
+                });
+            });
+        }
+
+        window.addEventListener('load', doTest, false);
+    </script>
+    <style>
+        body {
+            height: 2000px;
+        }
+        #target {
+            position: fixed;
+            top: 300px;
+            left: 20px;
+            width: 120px;
+            height: 100px;
+            background-color: gray;
+        }
+    </style>
+</head>
+<body>
+    
+    <div id="target" _onclick_="targetClicked()"></div>
+
+    <script src=""
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (227429 => 227430)


--- trunk/Source/WebCore/ChangeLog	2018-01-23 20:08:36 UTC (rev 227429)
+++ trunk/Source/WebCore/ChangeLog	2018-01-23 20:11:10 UTC (rev 227430)
@@ -1,3 +1,20 @@
+2018-01-23  Simon Fraser  <simon.fra...@apple.com>
+
+        Element with position:fixed stops scrolling at the bottom of the page, but is painted in the right place on Chacos.com.
+        https://bugs.webkit.org/show_bug.cgi?id=181741
+        rdar://problem/36593581
+
+        Reviewed by Tim Horton.
+
+        The #ifdef for iOS was wrong; on iOS, visibleSize() is in content coordinates and matches
+        unscaledDocumentRect, so there's no need to scale it. Doing so computed the wrong unscaledMaximumScrollPosition
+        which broke hit-testing when the document minimum scale was > 1.
+
+        Test: fast/visual-viewport/ios/min-scale-greater-than-one.html
+
+        * page/FrameView.cpp:
+        (WebCore::FrameView::unscaledMaximumScrollPosition const):
+
 2018-01-23  Antoine Quint  <grao...@apple.com>
 
         [Web Animations] Expose getKeyframes() and parsing of remaining keyframe properties

Modified: trunk/Source/WebCore/page/FrameView.cpp (227429 => 227430)


--- trunk/Source/WebCore/page/FrameView.cpp	2018-01-23 20:08:36 UTC (rev 227429)
+++ trunk/Source/WebCore/page/FrameView.cpp	2018-01-23 20:11:10 UTC (rev 227430)
@@ -1938,12 +1938,7 @@
     if (RenderView* renderView = this->renderView()) {
         IntRect unscaledDocumentRect = renderView->unscaledDocumentRect();
         unscaledDocumentRect.expand(0, headerHeight() + footerHeight());
-        IntSize visibleSize = this->visibleSize();
-#if PLATFORM(IOS)
-        // FIXME: visibleSize() is the unscaled size on macOS, but the scaled size on iOS. This should be consistent. webkit.org/b/174648.
-        visibleSize.scale(visibleContentScaleFactor());
-#endif        
-        ScrollPosition maximumPosition = ScrollPosition(unscaledDocumentRect.maxXMaxYCorner() - visibleSize).expandedTo({ 0, 0 });
+        ScrollPosition maximumPosition = ScrollPosition(unscaledDocumentRect.maxXMaxYCorner() - visibleSize()).expandedTo({ 0, 0 });
         if (frame().isMainFrame() && m_scrollPinningBehavior == PinToTop)
             maximumPosition.setY(unscaledMinimumScrollPosition().y());
 

Modified: trunk/Tools/ChangeLog (227429 => 227430)


--- trunk/Tools/ChangeLog	2018-01-23 20:08:36 UTC (rev 227429)
+++ trunk/Tools/ChangeLog	2018-01-23 20:11:10 UTC (rev 227430)
@@ -1,3 +1,17 @@
+2018-01-23  Simon Fraser  <simon.fra...@apple.com>
+
+        Element with position:fixed stops scrolling at the bottom of the page, but is painted in the right place on Chacos.com.
+        https://bugs.webkit.org/show_bug.cgi?id=181741
+        rdar://problem/36593581
+
+        Reviewed by Tim Horton.
+        
+        If zoomToScale:animated: is called with the current zoom level, call the callback
+        rather than doing nothing.
+
+        * WebKitTestRunner/cocoa/TestRunnerWKWebView.mm:
+        (-[TestRunnerWKWebView zoomToScale:animated:completionHandler:]):
+
 2018-01-23  Basuke Suzuki  <basuke.suz...@sony.com>
 
         Add ignore_errors keyword argument to Executive.run_command() for replacing the common pattern of error_handler usage

Modified: trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm (227429 => 227430)


--- trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm	2018-01-23 20:08:36 UTC (rev 227429)
+++ trunk/Tools/WebKitTestRunner/cocoa/TestRunnerWKWebView.mm	2018-01-23 20:11:10 UTC (rev 227430)
@@ -129,6 +129,13 @@
     ASSERT(!self.zoomToScaleCompletionHandler);
     self.zoomToScaleCompletionHandler = completionHandler;
 
+    if (self.scrollView.zoomScale == scale) {
+        dispatch_async(dispatch_get_main_queue(), ^{
+            completionHandler();
+        });
+        return;
+    }
+
     [self.scrollView setZoomScale:scale animated:animated];
 }
 
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to