- Revision
- 256513
- Author
- [email protected]
- Date
- 2020-02-13 09:20:42 -0800 (Thu, 13 Feb 2020)
Log Message
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.
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (256512 => 256513)
--- trunk/LayoutTests/ChangeLog 2020-02-13 17:11:28 UTC (rev 256512)
+++ trunk/LayoutTests/ChangeLog 2020-02-13 17:20:42 UTC (rev 256513)
@@ -1,3 +1,18 @@
+2020-02-13 Simon Fraser <[email protected]>
+
+ 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 <[email protected]>
Unreviewed, rolling out r255158, 255405 and r255486
Added: trunk/LayoutTests/fast/scrolling/programmatic-scroll-to-zero-zero-expected.html (0 => 256513)
--- trunk/LayoutTests/fast/scrolling/programmatic-scroll-to-zero-zero-expected.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/programmatic-scroll-to-zero-zero-expected.html 2020-02-13 17:20:42 UTC (rev 256513)
@@ -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>
Added: trunk/LayoutTests/fast/scrolling/programmatic-scroll-to-zero-zero.html (0 => 256513)
--- trunk/LayoutTests/fast/scrolling/programmatic-scroll-to-zero-zero.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/programmatic-scroll-to-zero-zero.html 2020-02-13 17:20:42 UTC (rev 256513)
@@ -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>
Modified: trunk/LayoutTests/platform/ios/TestExpectations (256512 => 256513)
--- trunk/LayoutTests/platform/ios/TestExpectations 2020-02-13 17:11:28 UTC (rev 256512)
+++ trunk/LayoutTests/platform/ios/TestExpectations 2020-02-13 17:20:42 UTC (rev 256513)
@@ -69,17 +69,7 @@
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-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 ]
Modified: trunk/Source/WebCore/ChangeLog (256512 => 256513)
--- trunk/Source/WebCore/ChangeLog 2020-02-13 17:11:28 UTC (rev 256512)
+++ trunk/Source/WebCore/ChangeLog 2020-02-13 17:20:42 UTC (rev 256513)
@@ -1,3 +1,21 @@
+2020-02-13 Simon Fraser <[email protected]>
+
+ 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 <[email protected]>
Unreviewed, rolling out r255158, 255405 and r255486
Modified: trunk/Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp (256512 => 256513)
--- trunk/Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp 2020-02-13 17:11:28 UTC (rev 256512)
+++ trunk/Source/WebCore/page/scrolling/ScrollingStateScrollingNode.cpp 2020-02-13 17:20:42 UTC (rev 256513)
@@ -220,9 +220,7 @@
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);
}
