Sideways jiggles when scrolling the shelves on beta.music.apple.com
https://bugs.webkit.org/show_bug.cgi?id=209696
<rdar://problem/55092050>
Reviewed by Anders Carlsson.
Source/WebCore:
If a scroll snapping animation was running, EventHandler::platformNotifyIfEndGesture() would
reset the latching state. This was added in r190423, but not longer seems necessary
according to manual testing, and the passing layout test.
platformNotifyIfEndGesture() would be called at the end of the fingers-down scroll but
before momentum, and resetting latching here would cause the momentum events to go to
a new target, triggering incorrect scrolls.
Test: tiled-drawing/scrolling/scroll-snap/scroll-snap-phase-change-relatching.html
* page/mac/EventHandlerMac.mm:
(WebCore::EventHandler::platformNotifyIfEndGesture):
LayoutTests:
Test that sends scroll and momentum events to a vertically-scrolling overflow with snap-points,
which checked that the document didn't scroll.
* tiled-drawing/scrolling/scroll-snap/scroll-snap-phase-change-relatching-expected.txt: Added.
* tiled-drawing/scrolling/scroll-snap/scroll-snap-phase-change-relatching.html: Added.
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@259162 268f45cc-cd09-0410-ab3c-d52691b4dbfc
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
index 3cb90ed..fd0f0ad 100644
--- a/LayoutTests/ChangeLog
+++ b/LayoutTests/ChangeLog
@@ -1,3 +1,17 @@
+2020-03-27 Simon Fraser <simon.fraser@apple.com>
+
+ Sideways jiggles when scrolling the shelves on beta.music.apple.com
+ https://bugs.webkit.org/show_bug.cgi?id=209696
+ <rdar://problem/55092050>
+
+ Reviewed by Anders Carlsson.
+
+ Test that sends scroll and momentum events to a vertically-scrolling overflow with snap-points,
+ which checked that the document didn't scroll.
+
+ * tiled-drawing/scrolling/scroll-snap/scroll-snap-phase-change-relatching-expected.txt: Added.
+ * tiled-drawing/scrolling/scroll-snap/scroll-snap-phase-change-relatching.html: Added.
+
2020-03-28 Antti Koivisto <antti@apple.com>
Nullptr crash in InlineTextBox::emphasisMarkExistsAndIsAbove
diff --git a/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-phase-change-relatching-expected.txt b/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-phase-change-relatching-expected.txt
new file mode 100644
index 0000000..6b22fd6
--- /dev/null
+++ b/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-phase-change-relatching-expected.txt
@@ -0,0 +1,11 @@
+Tests that latching is not recomputed between the scroll and momentum phases, which triggers a document scroll.
+
+On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE".
+
+
+PASS targetScroller.scrollTop is targetScroller.clientHeight
+PASS document.scrollingElement.scrollTop is 0
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-phase-change-relatching.html b/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-phase-change-relatching.html
new file mode 100644
index 0000000..eee85f0
--- /dev/null
+++ b/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-phase-change-relatching.html
@@ -0,0 +1,85 @@
+<!DOCTYPE html>
+<html>
+<head>
+ <style>
+ body {
+ height: 2000px;
+ }
+ #scroller {
+ width: 300px;
+ height: 300px;
+ overflow-x: hidden;
+ overflow-y: auto;
+ margin: 20px;
+ scroll-snap-type: y mandatory;
+ }
+ #scroller > div {
+ height: 100%;
+ width: 100%;
+ scroll-snap-align: start;
+ }
+ </style>
+ <script src="../../../resources/js-test.js"></script>
+ <script>
+ jsTestIsAsync = true;
+
+ var targetScroller;
+
+ function checkEndState()
+ {
+ shouldBe('targetScroller.scrollTop', 'targetScroller.clientHeight');
+ shouldBe('document.scrollingElement.scrollTop', '0');
+ finishJSTest();
+ }
+
+ function scrollTest()
+ {
+ targetScroller = document.getElementById('scroller');
+
+ var targetRect = targetScroller.getBoundingClientRect();
+
+ var dx = 0;
+ var dy = -1;
+ var startPosX = targetRect.left + 50;
+ var startPosY = targetRect.top + 50;
+ eventSender.monitorWheelEvents();
+ eventSender.mouseMoveTo(startPosX, startPosY);
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(dx, dy, 'began', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(dx, dy, 'changed', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(dx, dy, 'changed', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(dx, dy, 'changed', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'ended', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(dx, dy, 'none', 'begin');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(dx, dy, 'none', 'continue');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'none', 'end');
+ eventSender.callAfterScrollingCompletes(checkEndState);
+ }
+
+ function startTest()
+ {
+ description('Tests that latching is not recomputed between the scroll and momentum phases, which triggers a document scroll.');
+ if (window.eventSender) {
+ internals.setPlatformMomentumScrollingPredictionEnabled(false);
+ setTimeout(scrollTest, 0);
+ return;
+ }
+
+ debug('This test requires eventSender');
+ finishJSTest();
+ }
+
+ window.addEventListener('load', startTest, false);
+ </script>
+</head>
+<body>
+ <div id="scroller">
+ <div style="background-color: orange"></div>
+ <div style="background-color: green"></div>
+ <div style="background-color: blue"></div>
+ <div style="background-color: aqua"></div>
+ <div style="background-color: yellow"></div>
+ <div style="background-color: fuchsia"></div>
+ </div>
+ <div id="console"></div>
+</body>
+</html>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
index b50158d..1fd1a42 100644
--- a/Source/WebCore/ChangeLog
+++ b/Source/WebCore/ChangeLog
@@ -1,5 +1,26 @@
2020-03-27 Simon Fraser <simon.fraser@apple.com>
+ Sideways jiggles when scrolling the shelves on beta.music.apple.com
+ https://bugs.webkit.org/show_bug.cgi?id=209696
+ <rdar://problem/55092050>
+
+ Reviewed by Anders Carlsson.
+
+ If a scroll snapping animation was running, EventHandler::platformNotifyIfEndGesture() would
+ reset the latching state. This was added in r190423, but not longer seems necessary
+ according to manual testing, and the passing layout test.
+
+ platformNotifyIfEndGesture() would be called at the end of the fingers-down scroll but
+ before momentum, and resetting latching here would cause the momentum events to go to
+ a new target, triggering incorrect scrolls.
+
+ Test: tiled-drawing/scrolling/scroll-snap/scroll-snap-phase-change-relatching.html
+
+ * page/mac/EventHandlerMac.mm:
+ (WebCore::EventHandler::platformNotifyIfEndGesture):
+
+2020-03-27 Simon Fraser <simon.fraser@apple.com>
+
Define ENABLE_WHEEL_EVENT_LATCHING and use it to wrap wheel event latching code
https://bugs.webkit.org/show_bug.cgi?id=209693
diff --git a/Source/WebCore/page/mac/EventHandlerMac.mm b/Source/WebCore/page/mac/EventHandlerMac.mm
index 01b1fdb..1d63d3d 100644
--- a/Source/WebCore/page/mac/EventHandlerMac.mm
+++ b/Source/WebCore/page/mac/EventHandlerMac.mm
@@ -1105,11 +1105,8 @@
return;
#if ENABLE(CSS_SCROLL_SNAP)
- if (ScrollAnimator* scrollAnimator = scrollableArea->existingScrollAnimator()) {
+ if (ScrollAnimator* scrollAnimator = scrollableArea->existingScrollAnimator())
scrollAnimator->processWheelEventForScrollSnap(wheelEvent);
- if (scrollAnimator->isScrollSnapInProgress())
- clearLatchedState();
- }
#endif
}
