Add basic (non-momentum) wheel event handling for scroll snap

https://bugs.webkit.org/show_bug.cgi?id=222594 Reviewed by Darin Adler. Source/WebCore: Test: css3/scroll-snap/scroll-snap-wheel-event.html Enable scroll snapping for basic wheel events on GTK+ and WPE. The Mac port has special wheel handling due to momentum scrolling. Other scroll-snap-enabled ports can just use a basic version. * platform/ScrollAnimator.cpp: (WebCore::ScrollAnimator::scroll): Accept a bitmask of options now. This will allow using this method when handling wheel events that do not animate. (WebCore::ScrollAnimator::handleWheelEvent): Trigger ::scroll with scroll snapping enabled and pass the appropriate option to disable animations. (WebCore::ScrollAnimator::processWheelEventForScrollSnap): Deleted. * platform/ScrollAnimator.h: (WebCore::ScrollAnimator::ScrollAnimator::processWheelEventForScrollSnap): Made this a method that can be overridden by subclasses. * platform/mac/ScrollAnimatorMac.h: Added processWheelEventForScrollSnap. * platform/mac/ScrollAnimatorMac.mm: (WebCore::ScrollAnimatorMac::scroll): Pay attention to the NeverAnimate bitmask now. (WebCore::ScrollAnimatorMac::processWheelEventForScrollSnap): Added. LayoutTests: * css3/scroll-snap/scroll-snap-wheel-event-expected.txt: Added. * css3/scroll-snap/scroll-snap-wheel-event.html: Added. * platform/ios-wk2/TestExpectations: Skip new test because it uses mouse event simulation. Move existing classification to better section as well. * platform/mac-wk1/fast/scrolling/latching/scroll-snap-latching-expected.txt: Rebased this previous failing test. Canonical link: https://commits.webkit.org/236831@main git-svn-id: https://svn.webkit.org/repository/webkit/trunk@276353 268f45cc-cd09-0410-ab3c-d52691b4dbfc
WebKit · Apr 21, 2021 · 82dd9d2aa5249c6ec4922445a4e94bc935f92b95 · 82dd9d2
1 parent 913ca10
commit 82dd9d2aa5249c6ec4922445a4e94bc935f92b95
Showing 10 changed files with 161 additions and 36 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,16 @@
+2021-04-21  Martin Robinson  <mrobinson@webkit.org>
+
+        Add basic (non-momentum) wheel event handling for scroll snap
+        https://bugs.webkit.org/show_bug.cgi?id=222594
+
+        Reviewed by Darin Adler.
+
+        * css3/scroll-snap/scroll-snap-wheel-event-expected.txt: Added.
+        * css3/scroll-snap/scroll-snap-wheel-event.html: Added.
+        * platform/ios-wk2/TestExpectations: Skip new test because it uses mouse event simulation.
+        Move existing classification to better section as well.
+        * platform/mac-wk1/fast/scrolling/latching/scroll-snap-latching-expected.txt: Rebased this previous failing test.
+
 2021-04-20  Ian Gilbert  <iang@apple.com>
 
         Crash in CompositeEditCommand::insertNodeAt

diff --git a/LayoutTests/css3/scroll-snap/scroll-snap-wheel-event-expected.txt b/LayoutTests/css3/scroll-snap/scroll-snap-wheel-event-expected.txt
@@ -0,0 +1,6 @@
+PASS horizontal mouse wheel event snapped
+PASS vertical mouse wheel event snapped
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/css3/scroll-snap/scroll-snap-wheel-event.html b/LayoutTests/css3/scroll-snap/scroll-snap-wheel-event.html
@@ -0,0 +1,80 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <title>Simple wheel events should trigger scroll snapping</title>
+        <style type="text/css">
+            .container {
+                height: 200px;
+                width: 200px;
+                overflow: auto;
+                scroll-snap-type: both mandatory;
+            }
+
+            .horizontal-drawer {
+                height: 100%;
+                width: 500px;
+            }
+
+            .block {
+                height: 100%;
+                width: 200px;
+                scroll-snap-align: start;
+            }
+        </style>
+        <script src="../../resources/js-test.js"></script>
+        <script src="../../resources/ui-helper.js"></script>
+        <script>
+        window.jsTestIsAsync = true;
+
+        async function onLoad()
+        {
+            if (window.eventSender == undefined) {
+                document.getElementById('console').innerText = "Simple wheel events should trigger scroll snapping";
+                return;
+            }
+            try {
+                eventSender.mouseMoveTo(20, 190);
+                eventSender.mouseDown();
+                eventSender.mouseMoveTo(80, 190);
+                eventSender.mouseUp();
+
+                eventSender.mouseScrollBy(-1, 0);
+
+                let horizontalContainer = document.getElementById("horizontal-container");
+                await UIHelper.waitForTargetScrollAnimationToSettle(horizontalContainer);
+                expectTrue(horizontalContainer.scrollLeft == 200, "horizontal mouse wheel event snapped");
+
+                eventSender.mouseMoveTo(190, 220);
+                eventSender.mouseDown();
+                eventSender.mouseMoveTo(190, 270);
+                eventSender.mouseUp();
+
+                eventSender.mouseScrollBy(0, -1);
+
+                let verticalContainer = document.getElementById("vertical-container");
+                await UIHelper.waitForTargetScrollAnimationToSettle(verticalContainer);
+                expectTrue(verticalContainer.scrollTop == 185, "vertical mouse wheel event snapped");
+            } catch (e) {
+                console.log(e);
+            } finally {
+                finishJSTest();
+            }
+        }
+        </script>
+    </head>
+    <body onload="onLoad();">
+        <div id="horizontal-container" class="container">
+            <div class="horizontal-drawer">
+                <div class="block" style="float: left; background: #80475E"></div>
+                <div class="block" style="float: left; background: #CC5A71"></div>
+            </div>
+        </div>
+        <div id="vertical-container" class="container">
+            <div class="block" style="background: #80475E"></div>
+            <div class="block" style="background: #CC5A71"></div>
+        </div>
+        <p id="console"></p>
+        <script>
+        </script>
+    </body>
+</html>
diff --git a/LayoutTests/platform/ios-wk2/TestExpectations b/LayoutTests/platform/ios-wk2/TestExpectations
@@ -137,7 +137,6 @@ scrollbars/scrolling-by-page-on-keyboard-spacebar.html [ Failure ]
 scrollbars/scrollbars-on-positioned-content.html [ Failure ]
 css3/scroll-snap/scroll-padding-mainframe-paging.html [ Failure ]
 css3/scroll-snap/scroll-padding-overflow-paging.html [ Failure ]
-css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html [ Failure ]
 
 # SVG tests that time out (these require EventSender)
 svg/animations/animVal-basics.html
@@ -933,6 +932,8 @@ editing/selection/select-out-of-floated-non-editable-09.html [ Skip ]
 editing/selection/select-out-of-floated-non-editable-10.html [ Skip ]
 editing/selection/select-out-of-floated-non-editable-11.html [ Skip ]
 editing/selection/select-out-of-floated-non-editable-12.html [ Skip ]
+css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html [ Skip ]
+css3/scroll-snap/scroll-snap-wheel-event.html [ Skip ]
 
 webkit.org/b/192088 media/no-fullscreen-when-hidden.html [ Failure Pass ]
 

diff --git a/LayoutTests/platform/mac-wk1/fast/scrolling/latching/scroll-snap-latching-expected.txt b/LayoutTests/platform/mac-wk1/fast/scrolling/latching/scroll-snap-latching-expected.txt
@@ -4,7 +4,7 @@ On success, you will see a series of "PASS" messages, followed by "TEST COMPLETE
 
 
 PASS overflowScrollEventCount > 0 is true
-FAIL windowScrollEventCount should be 0. Was 7.
+FAIL windowScrollEventCount should be 0. Was 11.
 PASS successfullyParsed is true
 
 TEST COMPLETE

diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,30 @@
+2021-04-21  Martin Robinson  <mrobinson@webkit.org>
+
+        Add basic (non-momentum) wheel event handling for scroll snap
+        https://bugs.webkit.org/show_bug.cgi?id=222594
+
+        Reviewed by Darin Adler.
+
+        Test: css3/scroll-snap/scroll-snap-wheel-event.html
+
+        Enable scroll snapping for basic wheel events on GTK+ and WPE. The Mac port
+        has special wheel handling due to momentum scrolling. Other scroll-snap-enabled
+        ports can just use a basic version.
+
+        * platform/ScrollAnimator.cpp:
+        (WebCore::ScrollAnimator::scroll): Accept a bitmask of options now. This
+        will allow using this method when handling wheel events that do not animate.
+        (WebCore::ScrollAnimator::handleWheelEvent): Trigger ::scroll with
+        scroll snapping enabled and pass the appropriate option to disable animations.
+        (WebCore::ScrollAnimator::processWheelEventForScrollSnap): Deleted.
+        * platform/ScrollAnimator.h:
+        (WebCore::ScrollAnimator::ScrollAnimator::processWheelEventForScrollSnap): Made
+        this a method that can be overridden by subclasses.
+        * platform/mac/ScrollAnimatorMac.h: Added processWheelEventForScrollSnap.
+        * platform/mac/ScrollAnimatorMac.mm:
+        (WebCore::ScrollAnimatorMac::scroll): Pay attention to the NeverAnimate bitmask now.
+        (WebCore::ScrollAnimatorMac::processWheelEventForScrollSnap): Added.
+
 2021-04-21  Youenn Fablet  <youenn@apple.com>
 
         Use BlobURL::getOriginURL in more places

diff --git a/Source/WebCore/platform/ScrollAnimator.cpp b/Source/WebCore/platform/ScrollAnimator.cpp
@@ -81,13 +81,14 @@ ScrollAnimator::~ScrollAnimator()
 #endif
 }
 
-bool ScrollAnimator::scroll(ScrollbarOrientation orientation, ScrollGranularity granularity, float step, float multiplier, ScrollBehavior behavior)
+bool ScrollAnimator::scroll(ScrollbarOrientation orientation, ScrollGranularity granularity, float step, float multiplier, OptionSet<ScrollBehavior> behavior)
 {
     auto delta = deltaFromStep(orientation, step, multiplier);
 #if ENABLE(CSS_SCROLL_SNAP)
-    if (behavior == ScrollBehavior::DoDirectionalSnapping) {
+    if (behavior.contains(ScrollBehavior::DoDirectionalSnapping)) {
+        behavior.remove(ScrollBehavior::DoDirectionalSnapping);
         if (!m_scrollController.usesScrollSnap())
-            return scroll(orientation, granularity, step, multiplier);
+            return scroll(orientation, granularity, step, multiplier, behavior);
 
         auto currentOffset = offsetFromPosition(currentPosition());
         auto newOffset = currentOffset + delta;
@@ -98,16 +99,16 @@ bool ScrollAnimator::scroll(ScrollbarOrientation orientation, ScrollGranularity
         auto newDelta = newOffset - currentOffset;
 
         if (orientation == HorizontalScrollbar)
-            return scroll(HorizontalScrollbar, granularity, newDelta.width(), 1.0);
-        return scroll(VerticalScrollbar, granularity, newDelta.height(), 1.0);
+            return scroll(HorizontalScrollbar, granularity, newDelta.width(), 1.0, behavior);
+        return scroll(VerticalScrollbar, granularity, newDelta.height(), 1.0, behavior);
     }
 #else
     UNUSED_PARAM(granularity);
     UNUSED_PARAM(behavior);
 #endif
 
 #if ENABLE(SMOOTH_SCROLLING) && !PLATFORM(IOS_FAMILY)
-    if (m_scrollableArea.scrollAnimatorEnabled()) {
+    if (m_scrollableArea.scrollAnimatorEnabled() && !behavior.contains(ScrollBehavior::NeverAnimate)) {
         if (!m_scrollAnimation->isActive())
             m_scrollAnimation->setCurrentPosition(m_currentPosition);
         return m_scrollAnimation->scroll(orientation, granularity, step, multiplier);
@@ -177,13 +178,6 @@ FloatSize ScrollAnimator::deltaFromStep(ScrollbarOrientation orientation, float
 }
 
 #if ENABLE(CSS_SCROLL_SNAP)
-#if PLATFORM(MAC)
-bool ScrollAnimator::processWheelEventForScrollSnap(const PlatformWheelEvent& wheelEvent)
-{
-    return m_scrollController.processWheelEventForScrollSnap(wheelEvent);
-}
-#endif
-
 bool ScrollAnimator::activeScrollSnapIndexDidChange() const
 {
     return m_scrollController.activeScrollSnapIndexDidChange();
@@ -197,10 +191,11 @@ unsigned ScrollAnimator::activeScrollSnapIndexForAxis(ScrollEventAxis axis) cons
 
 bool ScrollAnimator::handleWheelEvent(const PlatformWheelEvent& e)
 {
-#if ENABLE(CSS_SCROLL_SNAP) && PLATFORM(MAC)
-    if (m_scrollController.processWheelEventForScrollSnap(e))
+#if ENABLE(CSS_SCROLL_SNAP)
+    if (processWheelEventForScrollSnap(e))
         return false;
 #endif
+
 #if PLATFORM(COCOA)
     // Events in the PlatformWheelEventPhase::MayBegin phase have no deltas, and therefore never passes through the scroll handling logic below.
     // This causes us to return with an 'unhandled' return state, even though this event was successfully processed.
@@ -221,7 +216,6 @@ bool ScrollAnimator::handleWheelEvent(const PlatformWheelEvent& e)
 
     bool handled = false;
 
-    ScrollGranularity granularity = ScrollByPixel;
     IntSize maxForwardScrollDelta = m_scrollableArea.maximumScrollPosition() - m_scrollableArea.scrollPosition();
     IntSize maxBackwardScrollDelta = m_scrollableArea.scrollPosition() - m_scrollableArea.minimumScrollPosition();
     if ((deltaX < 0 && maxForwardScrollDelta.width() > 0)
@@ -230,17 +224,18 @@ bool ScrollAnimator::handleWheelEvent(const PlatformWheelEvent& e)
         || (deltaY > 0 && maxBackwardScrollDelta.height() > 0)) {
         handled = true;
 
+        OptionSet<ScrollBehavior> behavior(ScrollBehavior::DoDirectionalSnapping);
+        if (e.hasPreciseScrollingDeltas())
+            behavior.add(ScrollBehavior::NeverAnimate);
+
         if (deltaY) {
             if (e.granularity() == ScrollByPageWheelEvent) {
                 bool negative = deltaY < 0;
                 deltaY = Scrollbar::pageStepDelta(m_scrollableArea.visibleHeight());
                 if (negative)
                     deltaY = -deltaY;
             }
-            if (e.hasPreciseScrollingDeltas())
-                scrollToPositionWithoutAnimation(currentPosition() + deltaFromStep(VerticalScrollbar, verticalScrollbar->pixelStep(), -deltaY));
-            else
-                scroll(VerticalScrollbar, granularity, verticalScrollbar->pixelStep(), -deltaY);
+            scroll(VerticalScrollbar, ScrollByPixel, verticalScrollbar->pixelStep(), -deltaY, behavior);
         }
 
         if (deltaX) {
@@ -250,10 +245,7 @@ bool ScrollAnimator::handleWheelEvent(const PlatformWheelEvent& e)
                 if (negative)
                     deltaX = -deltaX;
             }
-            if (e.hasPreciseScrollingDeltas())
-                scrollToPositionWithoutAnimation(currentPosition() + deltaFromStep(HorizontalScrollbar, horizontalScrollbar->pixelStep(), -deltaX));
-            else
-                scroll(HorizontalScrollbar, granularity, horizontalScrollbar->pixelStep(), -deltaX);
+            scroll(HorizontalScrollbar, ScrollByPixel, horizontalScrollbar->pixelStep(), -deltaX, behavior);
         }
     }
     return handled;

diff --git a/Source/WebCore/platform/ScrollAnimator.h b/Source/WebCore/platform/ScrollAnimator.h
@@ -69,15 +69,15 @@ class ScrollAnimator {
     virtual ~ScrollAnimator();
 
     enum ScrollBehavior {
-        Default,
-        DoDirectionalSnapping,
+        DoDirectionalSnapping = 1 << 0,
+        NeverAnimate = 1 << 1,
     };
 
     // Computes a scroll destination for the given parameters.  Returns false if
     // already at the destination.  Otherwise, starts scrolling towards the
     // destination and returns true.  Scrolling may be immediate or animated.
     // The base class implementation always scrolls immediately, never animates.
-    virtual bool scroll(ScrollbarOrientation, ScrollGranularity, float step, float multiplier, ScrollBehavior = ScrollBehavior::Default);
+    virtual bool scroll(ScrollbarOrientation, ScrollGranularity, float step, float multiplier, OptionSet<ScrollBehavior>);
 
     bool scrollToOffsetWithoutAnimation(const FloatPoint&, ScrollClamping = ScrollClamping::Clamped);
     virtual bool scrollToPositionWithoutAnimation(const FloatPoint& position, ScrollClamping = ScrollClamping::Clamped);
@@ -149,9 +149,7 @@ class ScrollAnimator {
 #endif
 
 #if ENABLE(CSS_SCROLL_SNAP)
-#if PLATFORM(MAC)
-    bool processWheelEventForScrollSnap(const PlatformWheelEvent&);
-#endif
+    virtual bool processWheelEventForScrollSnap(const PlatformWheelEvent&) { return false; }
     void updateScrollSnapState();
     bool activeScrollSnapIndexDidChange() const;
     unsigned activeScrollSnapIndexForAxis(ScrollEventAxis) const;

diff --git a/Source/WebCore/platform/mac/ScrollAnimatorMac.h b/Source/WebCore/platform/mac/ScrollAnimatorMac.h
@@ -79,7 +79,7 @@ class ScrollAnimatorMac : public ScrollAnimator {
     Timer m_sendContentAreaScrolledTimer;
     FloatSize m_contentAreaScrolledTimerScrollDelta;
 
-    bool scroll(ScrollbarOrientation, ScrollGranularity, float step, float multiplier, ScrollBehavior) override;
+    bool scroll(ScrollbarOrientation, ScrollGranularity, float step, float multiplier, OptionSet<ScrollBehavior>) override;
     bool scrollToPositionWithAnimation(const FloatPoint&) override;
     bool scrollToPositionWithoutAnimation(const FloatPoint& position, ScrollClamping = ScrollClamping::Clamped) override;
 
@@ -140,6 +140,8 @@ class ScrollAnimatorMac : public ScrollAnimator {
     String horizontalScrollbarStateForTesting() const final;
     String verticalScrollbarStateForTesting() const final;
 
+    bool processWheelEventForScrollSnap(const PlatformWheelEvent&) override;
+
     // ScrollControllerClient.
 #if ENABLE(RUBBER_BANDING)
     IntSize stretchAmount() const final;

diff --git a/Source/WebCore/platform/mac/ScrollAnimatorMac.mm b/Source/WebCore/platform/mac/ScrollAnimatorMac.mm
@@ -751,16 +751,17 @@ static bool rubberBandingEnabledForSystem()
 }
 #endif
 
-bool ScrollAnimatorMac::scroll(ScrollbarOrientation orientation, ScrollGranularity granularity, float step, float multiplier, ScrollBehavior behavior)
+bool ScrollAnimatorMac::scroll(ScrollbarOrientation orientation, ScrollGranularity granularity, float step, float multiplier, OptionSet<ScrollBehavior> behavior)
 {
     m_haveScrolledSincePageLoad = true;
 
     // This method doesn't do directional snapping, but our base class does. It will call into
     // ScrollAnimatorMac::scroll again with the snapped positions and ScrollBehavior::Default.
-    if (behavior == ScrollBehavior::DoDirectionalSnapping)
+    if (behavior.contains(ScrollBehavior::DoDirectionalSnapping))
         return ScrollAnimator::scroll(orientation, granularity, step, multiplier, behavior);
 
-    bool shouldAnimate = scrollAnimationEnabledForSystem() && m_scrollableArea.scrollAnimatorEnabled() && granularity != ScrollByPixel;
+    bool shouldAnimate = scrollAnimationEnabledForSystem() && m_scrollableArea.scrollAnimatorEnabled() && granularity != ScrollByPixel
+        && !behavior.contains(ScrollBehavior::NeverAnimate);
     FloatPoint newPosition = this->currentPosition() + deltaFromStep(orientation, step, multiplier);
     newPosition = newPosition.constrainedBetween(scrollableArea().minimumScrollPosition(), scrollableArea().maximumScrollPosition());
 
@@ -1500,6 +1501,11 @@ static bool gestureShouldBeginSnap(const PlatformWheelEvent& wheelEvent, ScrollE
     m_visibleScrollerThumbRect = rectInViewCoordinates;
 }
 
+bool ScrollAnimatorMac::processWheelEventForScrollSnap(const PlatformWheelEvent& wheelEvent)
+{
+    return m_scrollController.processWheelEventForScrollSnap(wheelEvent);
+}
+
 } // namespace WebCore
 
 #endif // ENABLE(SMOOTH_SCROLLING)