Diff
Modified: trunk/LayoutTests/ChangeLog (283625 => 283626)
--- trunk/LayoutTests/ChangeLog 2021-10-06 15:57:51 UTC (rev 283625)
+++ trunk/LayoutTests/ChangeLog 2021-10-06 16:06:36 UTC (rev 283626)
@@ -1,3 +1,18 @@
+2021-10-06 Chris Lord <[email protected]>
+
+ [GTK][WPE] CSS scroll snapping interacts badly with smooth scrolling
+ https://bugs.webkit.org/show_bug.cgi?id=229037
+
+ Reviewed by Simon Fraser.
+
+ Add a GTK-only test to verify CSS scroll snapping and kinetic/touchpad
+ scrolling behave correctly.
+
+ * TestExpectations:
+ * fast/scrolling/gtk/user-scroll-snapping-interaction-expected.txt: Added.
+ * fast/scrolling/gtk/user-scroll-snapping-interaction.html: Added.
+ * platform/gtk/TestExpectations:
+
2021-10-06 Ayumi Kojima <[email protected]>
[ iOS 15 ] 2 http/tests/app-privacy-report tests are timing out.
Modified: trunk/LayoutTests/TestExpectations (283625 => 283626)
--- trunk/LayoutTests/TestExpectations 2021-10-06 15:57:51 UTC (rev 283625)
+++ trunk/LayoutTests/TestExpectations 2021-10-06 16:06:36 UTC (rev 283626)
@@ -59,6 +59,7 @@
fast/events/pointer/ios [ Skip ]
fast/events/touch/ios [ Skip ]
fast/history/ios [ Skip ]
+fast/scrolling/gtk [ Skip ]
fast/scrolling/ios [ Skip ]
fast/scrolling/mac [ Skip ]
fast/scrolling/ipad [ Skip ]
Added: trunk/LayoutTests/fast/scrolling/gtk/user-scroll-snapping-interaction-expected.txt (0 => 283626)
--- trunk/LayoutTests/fast/scrolling/gtk/user-scroll-snapping-interaction-expected.txt (rev 0)
+++ trunk/LayoutTests/fast/scrolling/gtk/user-scroll-snapping-interaction-expected.txt 2021-10-06 16:06:36 UTC (rev 283626)
@@ -0,0 +1,9 @@
+PASS Scroll downwards by 25 pixels
+PASS Scroll downwards by another 25 pixels
+PASS End scroll snaps back to top
+PASS Scroll downwards by 200 pixels
+PASS End scroll should snap to next element
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/fast/scrolling/gtk/user-scroll-snapping-interaction.html (0 => 283626)
--- trunk/LayoutTests/fast/scrolling/gtk/user-scroll-snapping-interaction.html (rev 0)
+++ trunk/LayoutTests/fast/scrolling/gtk/user-scroll-snapping-interaction.html 2021-10-06 16:06:36 UTC (rev 283626)
@@ -0,0 +1,120 @@
+<!-- webkit-test-runner [ useThreadedScrolling=false ] -->
+<!DOCTYPE html>
+
+<html>
+<head>
+ <script src=""
+ <title>CSS scroll snapping should not conflict with user scrolling</title>
+ <style>
+ html {
+ scroll-snap-type: y mandatory;
+ }
+
+ body {
+ margin: 0;
+ }
+
+ div {
+ width: 100%;
+ height: 250px;
+ scroll-snap-align: start;
+ }
+ </style>
+</head>
+<body>
+ <div></div>
+ <div></div>
+ <div></div>
+ <div></div>
+ <div></div>
+ <div></div>
+ <div></div>
+ <div></div>
+ <div></div>
+ <div></div>
+</body>
+<script>
+var expectedScrollTop = 0;
+var testsPassed = 0;
+var testDescription;
+
+function startScroll(startx, starty, deltaX, deltaY)
+{
+ eventSender.mouseMoveTo(startx, starty);
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "began", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(-deltaX, -deltaY, "changed", "none");
+}
+
+function continueScroll(deltaX, deltaY)
+{
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(-deltaX, -deltaY, "changed", "none");
+}
+
+function endScroll(deltaX, deltaY)
+{
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(-deltaX, -deltaY, "ended", "none");
+}
+
+function triggerNextTest() {
+ if (testsPassed)
+ debug("PASS " + testDescription);
+
+ switch (testsPassed) {
+ case 0:
+ testDescription = "Scroll downwards by 25 pixels"
+ expectedScrollTop = 25;
+ startScroll(50, 50, 0, 25);
+ return;
+
+ case 1:
+ testDescription = "Scroll downwards by another 25 pixels"
+ expectedScrollTop = 50;
+ continueScroll(0, 25);
+ return;
+
+ case 2:
+ testDescription = "End scroll snaps back to top"
+ expectedScrollTop = 0;
+ endScroll(0, 0);
+ return;
+
+ case 3:
+ testDescription = "Scroll downwards by 200 pixels"
+ expectedScrollTop = 200;
+ startScroll(50, 50, 0, 200);
+ return;
+
+ case 4:
+ testDescription = "End scroll should snap to next element"
+ expectedScrollTop = 250;
+ endScroll(0, 0);
+ return;
+ }
+
+ isSuccessfullyParsed();
+ testRunner.notifyDone();
+
+ return;
+}
+
+function scrollEventCallback() {
+ if (document.scrollingElement.scrollTop == expectedScrollTop) {
+ ++testsPassed;
+ if (window.testRunner)
+ triggerNextTest();
+ }
+}
+
+
+document.addEventListener("scroll", scrollEventCallback, false);
+
+if (window.testRunner) {
+ testRunner.waitUntilDone();
+}
+
+if (window.eventSender) {
+ eventSender.setWheelHasPreciseDeltas(true);
+ triggerNextTest();
+}
+</script>
+</html>
Modified: trunk/LayoutTests/platform/gtk/TestExpectations (283625 => 283626)
--- trunk/LayoutTests/platform/gtk/TestExpectations 2021-10-06 15:57:51 UTC (rev 283625)
+++ trunk/LayoutTests/platform/gtk/TestExpectations 2021-10-06 16:06:36 UTC (rev 283626)
@@ -19,6 +19,7 @@
# Platform-specific directories.
accessibility/gtk [ Pass ]
editing/pasteboard/gtk [ Pass ]
+fast/scrolling/gtk [ Pass ]
swipe [ Pass ]
ietestcenter/css3/flexbox/flexbox-layout-001.htm [ Pass ]
Modified: trunk/Source/WebCore/ChangeLog (283625 => 283626)
--- trunk/Source/WebCore/ChangeLog 2021-10-06 15:57:51 UTC (rev 283625)
+++ trunk/Source/WebCore/ChangeLog 2021-10-06 16:06:36 UTC (rev 283626)
@@ -1,3 +1,30 @@
+2021-10-06 Chris Lord <[email protected]>
+
+ [GTK][WPE] CSS scroll snapping interacts badly with smooth scrolling
+ https://bugs.webkit.org/show_bug.cgi?id=229037
+
+ Reviewed by Simon Fraser.
+
+ Make CSS scroll snapping behave in a predictable way when scrolling
+ with touchpads/screens on GTK/WPE.
+
+ Test: fast/scrolling/gtk/user-scroll-snapping-interaction.html
+
+ * platform/ScrollAnimator.h:
+ (WebCore::ScrollAnimator::isUserScrollInProgress const):
+ (WebCore::ScrollAnimator::isScrollSnapInProgress const):
+ * platform/ScrollingEffectsController.cpp:
+ (WebCore::ScrollingEffectsController::processWheelEventForKineticScrolling):
+ (WebCore::ScrollingEffectsController::adjustDeltaForSnappingIfNeeded):
+ (WebCore::ScrollingEffectsController::handleWheelEvent):
+ (WebCore::ScrollingEffectsController::isUserScrollInProgress const):
+ (WebCore::ScrollingEffectsController::isScrollSnapInProgress const):
+ * platform/ScrollingEffectsController.h:
+ * platform/mac/ScrollAnimatorMac.h:
+ * platform/mac/ScrollAnimatorMac.mm:
+ (WebCore::ScrollAnimatorMac::isUserScrollInProgress const): Deleted.
+ (WebCore::ScrollAnimatorMac::isScrollSnapInProgress const): Deleted.
+
2021-10-06 Antti Koivisto <[email protected]>
[LFC] Layout box should own its children
Modified: trunk/Source/WebCore/platform/ScrollAnimator.h (283625 => 283626)
--- trunk/Source/WebCore/platform/ScrollAnimator.h 2021-10-06 15:57:51 UTC (rev 283625)
+++ trunk/Source/WebCore/platform/ScrollAnimator.h 2021-10-06 16:06:36 UTC (rev 283626)
@@ -93,10 +93,11 @@
virtual void cancelAnimations();
- virtual bool isUserScrollInProgress() const { return false; }
virtual bool isRubberBandInProgress() const { return false; }
- virtual bool isScrollSnapInProgress() const { return false; }
+ bool isUserScrollInProgress() const { return m_scrollController.isUserScrollInProgress(); }
+ bool isScrollSnapInProgress() const { return m_scrollController.isScrollSnapInProgress(); }
+
void contentsSizeChanged();
enum NotifyScrollableArea : bool {
Modified: trunk/Source/WebCore/platform/ScrollingEffectsController.cpp (283625 => 283626)
--- trunk/Source/WebCore/platform/ScrollingEffectsController.cpp 2021-10-06 15:57:51 UTC (rev 283625)
+++ trunk/Source/WebCore/platform/ScrollingEffectsController.cpp 2021-10-06 16:06:36 UTC (rev 283626)
@@ -240,11 +240,16 @@
if (!event.isEndOfNonMomentumScroll() && !event.isTransitioningToMomentumScroll())
return false;
+ m_inScrollGesture = false;
+
if (m_currentAnimation && !is<ScrollAnimationKinetic>(m_currentAnimation.get())) {
m_currentAnimation->stop();
m_currentAnimation = nullptr;
}
+ if (usesScrollSnap())
+ return false;
+
if (!m_currentAnimation)
m_currentAnimation = makeUnique<ScrollAnimationKinetic>(*this);
@@ -267,9 +272,29 @@
}
#endif
+void ScrollingEffectsController::adjustDeltaForSnappingIfNeeded(float& deltaX, float& deltaY)
+{
+ if (snapOffsetsInfo() && !snapOffsetsInfo()->isEmpty()) {
+ float scale = m_client.pageScaleFactor();
+ auto scrollOffset = m_client.scrollOffset();
+ auto extents = m_client.scrollExtents();
+
+ auto originalOffset = LayoutPoint(scrollOffset.x() / scale, scrollOffset.y() / scale);
+ auto newOffset = LayoutPoint((scrollOffset.x() + deltaX) / scale, (scrollOffset.y() + deltaY) / scale);
+
+ auto offsetX = snapOffsetsInfo()->closestSnapOffset(ScrollEventAxis::Horizontal, LayoutSize(extents.contentsSize), newOffset, deltaX, originalOffset.x()).first;
+ auto offsetY = snapOffsetsInfo()->closestSnapOffset(ScrollEventAxis::Vertical, LayoutSize(extents.contentsSize), newOffset, deltaY, originalOffset.y()).first;
+
+ deltaX = (offsetX - originalOffset.x()) * scale;
+ deltaY = (offsetY - originalOffset.y()) * scale;
+ }
+}
+
bool ScrollingEffectsController::handleWheelEvent(const PlatformWheelEvent& wheelEvent)
{
#if ENABLE(KINETIC_SCROLLING)
+ m_inScrollGesture = wheelEvent.hasPreciseScrollingDeltas() && !wheelEvent.isEndOfNonMomentumScroll() && !wheelEvent.isTransitioningToMomentumScroll();
+
if (processWheelEventForKineticScrolling(wheelEvent))
return true;
#endif
@@ -288,9 +313,6 @@
|| (deltaY > 0 && scrollOffset.y() <= minPosition.y()))
deltaY = 0;
- if (!deltaX && !deltaY)
- return false;
-
if (wheelEvent.granularity() == ScrollByPageWheelEvent) {
if (deltaX) {
bool negative = deltaX < 0;
@@ -309,20 +331,14 @@
deltaX = -deltaX;
deltaY = -deltaY;
- if (snapOffsetsInfo() && !snapOffsetsInfo()->isEmpty()) {
- float scale = m_client.pageScaleFactor();
- auto originalOffset = LayoutPoint(scrollOffset.x() / scale, scrollOffset.y() / scale);
- auto newOffset = LayoutPoint((scrollOffset.x() + deltaX) / scale, (scrollOffset.y() + deltaY) / scale);
+ if (!m_inScrollGesture)
+ adjustDeltaForSnappingIfNeeded(deltaX, deltaY);
- auto offsetX = snapOffsetsInfo()->closestSnapOffset(ScrollEventAxis::Horizontal, LayoutSize(extents.contentsSize), newOffset, deltaX, originalOffset.x()).first;
- auto offsetY = snapOffsetsInfo()->closestSnapOffset(ScrollEventAxis::Vertical, LayoutSize(extents.contentsSize), newOffset, deltaY, originalOffset.y()).first;
+ if (!deltaX && !deltaY)
+ return false;
- deltaX = (offsetX - originalOffset.x()) * scale;
- deltaY = (offsetY - originalOffset.y()) * scale;
- }
-
#if ENABLE(SMOOTH_SCROLLING)
- if (m_client.scrollAnimationEnabled() && !wheelEvent.hasPreciseScrollingDeltas()) {
+ if (m_client.scrollAnimationEnabled() && !m_inScrollGesture) {
if (is<ScrollAnimationSmooth>(m_currentAnimation.get())) {
auto lastDestinationOffset = downcast<ScrollAnimationSmooth>(*m_currentAnimation).destinationOffset();
retargetAnimatedScroll(lastDestinationOffset + FloatSize { deltaX, deltaY });
@@ -336,6 +352,22 @@
return true;
}
+
+bool ScrollingEffectsController::isUserScrollInProgress() const
+{
+ return m_inScrollGesture;
+}
+
+bool ScrollingEffectsController::isScrollSnapInProgress() const
+{
+ if (!usesScrollSnap())
+ return false;
+
+ if (m_inScrollGesture || (m_currentAnimation && m_currentAnimation->isActive()))
+ return true;
+
+ return false;
+}
#endif
void ScrollingEffectsController::updateActiveScrollSnapIndexForClientOffset()
Modified: trunk/Source/WebCore/platform/ScrollingEffectsController.h (283625 => 283626)
--- trunk/Source/WebCore/platform/ScrollingEffectsController.h 2021-10-06 15:57:51 UTC (rev 283625)
+++ trunk/Source/WebCore/platform/ScrollingEffectsController.h 2021-10-06 16:06:36 UTC (rev 283626)
@@ -163,12 +163,12 @@
// Returns true if handled.
bool handleWheelEvent(const PlatformWheelEvent&);
+ bool isScrollSnapInProgress() const;
+ bool isUserScrollInProgress() const;
+
#if PLATFORM(MAC)
static FloatSize wheelDeltaBiasingTowardsVertical(const PlatformWheelEvent&);
- bool isScrollSnapInProgress() const;
- bool isUserScrollInProgress() const;
-
// Returns true if handled.
bool processWheelEventForScrollSnap(const PlatformWheelEvent&);
@@ -216,6 +216,8 @@
void scrollAnimationDidEnd(ScrollAnimation&) final;
ScrollExtents scrollExtentsForAnimation(ScrollAnimation&) final;
+ void adjustDeltaForSnappingIfNeeded(float& deltaX, float& deltaY);
+
#if ENABLE(KINETIC_SCROLLING) && !PLATFORM(MAC)
// Returns true if handled.
bool processWheelEventForKineticScrolling(const PlatformWheelEvent&);
@@ -234,6 +236,7 @@
bool m_isAnimatingRubberBand { false };
bool m_isAnimatingScrollSnap { false };
bool m_isAnimatingKeyboardScrolling { false };
+ bool m_inScrollGesture { false };
#if PLATFORM(MAC)
WallTime m_lastMomentumScrollTimestamp;
@@ -241,7 +244,6 @@
FloatSize m_stretchScrollForce;
FloatSize m_momentumVelocity;
- bool m_inScrollGesture { false };
bool m_momentumScrollInProgress { false };
bool m_ignoreMomentumScrolls { false };
bool m_isRubberBanding { false };
Modified: trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.h (283625 => 283626)
--- trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.h 2021-10-06 15:57:51 UTC (rev 283625)
+++ trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.h 2021-10-06 16:06:36 UTC (rev 283626)
@@ -49,10 +49,8 @@
bool platformAllowsScrollAnimation() const;
void handleWheelEventPhase(PlatformWheelEventPhase) final;
-
- bool isUserScrollInProgress() const final;
+
bool isRubberBandInProgress() const final;
- bool isScrollSnapInProgress() const final;
bool processWheelEventForScrollSnap(const PlatformWheelEvent&) final;
Modified: trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm (283625 => 283626)
--- trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm 2021-10-06 15:57:51 UTC (rev 283625)
+++ trunk/Source/WebCore/platform/mac/ScrollAnimatorMac.mm 2021-10-06 16:06:36 UTC (rev 283626)
@@ -73,21 +73,11 @@
return enabled;
}
-bool ScrollAnimatorMac::isUserScrollInProgress() const
-{
- return m_scrollController.isUserScrollInProgress();
-}
-
bool ScrollAnimatorMac::isRubberBandInProgress() const
{
return m_scrollController.isRubberBandInProgress();
}
-bool ScrollAnimatorMac::isScrollSnapInProgress() const
-{
- return m_scrollController.isScrollSnapInProgress();
-}
-
void ScrollAnimatorMac::handleWheelEventPhase(PlatformWheelEventPhase phase)
{
LOG_WITH_STREAM(OverlayScrollbars, stream << "ScrollAnimatorMac " << this << " scrollableArea " << m_scrollableArea << " handleWheelEventPhase " << phase);
