REGRESSION (r255037): Zooming in and out on Quip in macOS Safari can cause the conten...
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Feb 2020 17:20:42 +0000 (17:20 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 13 Feb 2020 17:20:42 +0000 (17:20 +0000)
https://bugs.webkit.org/show_bug.cgi?id=207674
rdar://problem/59404866

Reviewed by Antti Koivisto.

Source/WebCore:

Remove the early return in ScrollingStateScrollingNode::setRequestedScrollData(); comparing
with the last m_requestedScrollData is wrong, because requested scroll positions are not "state"
in the scrolling tree, they are requests to scroll. Ideally, they would be represented in some
different way in the scrolling tree.

Test: fast/scrolling/programmatic-scroll-to-zero-zero.html

* page/scrolling/ScrollingStateScrollingNode.cpp:
(WebCore::ScrollingStateScrollingNode::setRequestedScrollData):

LayoutTests:

Test that does a programmatic scroll to 0,0, does a user scroll, then a second programmatic scroll to 0,0,
which is expected to work.

* fast/scrolling/programmatic-scroll-to-zero-zero-expected.html: Added.
* fast/scrolling/programmatic-scroll-to-zero-zero.html: Added.
* platform/ios/TestExpectations: Skip the new test on iOS (it relies on eventSender) and sort the grouping.

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

LayoutTests/ChangeLog
LayoutTests/fast/scrolling/programmatic-scroll-to-zero-zero-expected.html [new file with mode: 0644]
LayoutTests/fast/scrolling/programmatic-scroll-to-zero-zero.html [new file with mode: 0644]
LayoutTests/platform/ios/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp

index a67f2ed..3729a33 100644 (file)
@@ -1,3 +1,18 @@
+2020-02-13  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (r255037): Zooming in and out on Quip in macOS Safari can cause the content to be offset to the side
+        https://bugs.webkit.org/show_bug.cgi?id=207674
+        rdar://problem/59404866
+
+        Reviewed by Antti Koivisto.
+        
+        Test that does a programmatic scroll to 0,0, does a user scroll, then a second programmatic scroll to 0,0,
+        which is expected to work.
+
+        * fast/scrolling/programmatic-scroll-to-zero-zero-expected.html: Added.
+        * fast/scrolling/programmatic-scroll-to-zero-zero.html: Added.
+        * platform/ios/TestExpectations: Skip the new test on iOS (it relies on eventSender) and sort the grouping.
+
 2020-02-13  Said Abou-Hallawa  <said@apple.com>
 
         Unreviewed, rolling out r255158, 255405 and r255486
diff --git a/LayoutTests/fast/scrolling/programmatic-scroll-to-zero-zero-expected.html b/LayoutTests/fast/scrolling/programmatic-scroll-to-zero-zero-expected.html
new file mode 100644 (file)
index 0000000..8d547d7
--- /dev/null
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>The page should scroll back to the top</title>
+    <style>
+        body {
+            height: 2000px;
+            background-image: repeating-linear-gradient(white, silver 200px);
+        }
+        
+        .box {
+            width: 100px;
+            height: 100px;
+            background-color: green;
+        }
+    </style>
+</head>
+<body>
+    <div class="box"></div>
+</body>
+</html>
diff --git a/LayoutTests/fast/scrolling/programmatic-scroll-to-zero-zero.html b/LayoutTests/fast/scrolling/programmatic-scroll-to-zero-zero.html
new file mode 100644 (file)
index 0000000..2a21abc
--- /dev/null
@@ -0,0 +1,46 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>The page should scroll back to the top</title>
+    <style>
+        body {
+            height: 2000px;
+            background-image: repeating-linear-gradient(white, silver 200px);
+        }
+        
+        .box {
+            width: 100px;
+            height: 100px;
+            background-color: green;
+        }
+    </style>
+    <script>
+               if (window.testRunner)
+            testRunner.waitUntilDone();
+
+               function doTest()
+        {
+            document.scrollingElement.scrollTop = 20;
+            document.scrollingElement.scrollTop = 0;
+            
+            eventSender.monitorWheelEvents();
+
+            setTimeout(() => {
+                eventSender.mouseMoveTo(20, 20);
+                eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "began", "none");
+                eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -10, "changed", "none");
+                eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "ended", "none");
+                eventSender.callAfterScrollingCompletes(() => {
+                    document.scrollingElement.scrollTop = 0;
+                    testRunner.notifyDone();
+                });
+            }, 0);
+        }
+
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div class="box"></div>
+</body>
+</html>
index f29cf1f..cc65eec 100644 (file)
@@ -69,17 +69,7 @@ fast/history/page-cache-running-audiocontext.html
 fast/history/page-cache-suspended-audiocontext.html
 
 # No wheel events on iOS
-fast/scrolling/iframe-scrollable-after-back.html [ Skip ]
-fast/scrolling/overflow-scrollable-after-back.html [ Skip ]
-fast/events/wheel-event-destroys-frame.html [ Skip ]
-fast/events/wheel-event-destroys-overflow.html [ Skip ]
-fast/events/wheel-event-outside-body.html [ Skip ]
-fast/scrolling/scroll-container-horizontally.html [ Failure ]
-fast/events/wheelevent-basic.html [ Skip ]
-fast/events/wheelevent-direction-inverted-from-device.html [ Skip ]
-fast/events/wheelevent-in-horizontal-scrollbar-in-rtl.html [ Skip ]
-fast/events/wheelevent-in-vertical-scrollbar-in-rtl.html [ Skip ]
-fast/events/wheelevent-mousewheel-interaction.html [ Skip ]
+fast/events/continuous-platform-wheelevent-in-scrolling-div.html [ Skip ]
 fast/events/platform-wheelevent-in-scrolling-div.html [ Skip ]
 fast/events/platform-wheelevent-paging-x-in-non-scrolling-div.html [ Skip ]
 fast/events/platform-wheelevent-paging-x-in-non-scrolling-page.html [ Skip ]
@@ -91,8 +81,19 @@ fast/events/platform-wheelevent-paging-y-in-non-scrolling-div.html [ Skip ]
 fast/events/platform-wheelevent-paging-y-in-non-scrolling-page.html [ Skip ]
 fast/events/platform-wheelevent-paging-y-in-scrolling-div.html [ Skip ]
 fast/events/platform-wheelevent-paging-y-in-scrolling-page.html [ Skip ]
+fast/events/wheel-event-destroys-frame.html [ Skip ]
+fast/events/wheel-event-destroys-overflow.html [ Skip ]
+fast/events/wheel-event-outside-body.html [ Skip ]
+fast/events/wheelevent-basic.html [ Skip ]
+fast/events/wheelevent-direction-inverted-from-device.html [ Skip ]
+fast/events/wheelevent-in-horizontal-scrollbar-in-rtl.html [ Skip ]
 fast/events/wheelevent-in-text-node.html [ Skip ]
-fast/events/continuous-platform-wheelevent-in-scrolling-div.html [ Skip ]
+fast/events/wheelevent-in-vertical-scrollbar-in-rtl.html [ Skip ]
+fast/events/wheelevent-mousewheel-interaction.html [ Skip ]
+fast/scrolling/iframe-scrollable-after-back.html [ Skip ]
+fast/scrolling/overflow-scrollable-after-back.html [ Skip ]
+fast/scrolling/programmatic-scroll-to-zero-zero.html [ Skip ]
+fast/scrolling/scroll-container-horizontally.html [ Failure ]
 
 # This test requires alpha-channel video support.
 compositing/video/video-background-color.html [ WontFix ]
index 0cc7491..111880d 100644 (file)
@@ -1,3 +1,21 @@
+2020-02-13  Simon Fraser  <simon.fraser@apple.com>
+
+        REGRESSION (r255037): Zooming in and out on Quip in macOS Safari can cause the content to be offset to the side
+        https://bugs.webkit.org/show_bug.cgi?id=207674
+        rdar://problem/59404866
+
+        Reviewed by Antti Koivisto.
+        
+        Remove the early return in ScrollingStateScrollingNode::setRequestedScrollData(); comparing
+        with the last m_requestedScrollData is wrong, because requested scroll positions are not "state"
+        in the scrolling tree, they are requests to scroll. Ideally, they would be represented in some
+        different way in the scrolling tree.
+
+        Test: fast/scrolling/programmatic-scroll-to-zero-zero.html
+
+        * page/scrolling/ScrollingStateScrollingNode.cpp:
+        (WebCore::ScrollingStateScrollingNode::setRequestedScrollData):
+
 2020-02-13  Said Abou-Hallawa  <said@apple.com>
 
         Unreviewed, rolling out r255158, 255405 and r255486
index 92fbebc..fa3acb0 100644 (file)
@@ -220,9 +220,7 @@ void ScrollingStateScrollingNode::setScrollableAreaParameters(const ScrollableAr
 
 void ScrollingStateScrollingNode::setRequestedScrollData(const RequestedScrollData& scrollData)
 {
-    if (scrollData == m_requestedScrollData)
-        return;
-
+    // Scroll position requests are imperative, not stateful, so we can't early return here.
     m_requestedScrollData = scrollData;
     setPropertyChanged(RequestedScrollPosition);
 }