scroll-snap-points do not work very well with mechanical scroll wheel events
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Jul 2015 01:51:16 +0000 (01:51 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 1 Jul 2015 01:51:16 +0000 (01:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=142501
<rdar://problem/20093511>

Patch by Wenson Hsieh <whsieh@berkeley.edu> on 2015-06-30
Reviewed by Brent Fulgham.

Source/WebCore:

Stateless scroll events generated by scrolling with a mechanical mouse wheel now trigger scroll
snapping after a fixed delay.

Test: platform/mac-wk2/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-overflow-stateless.html

* platform/cocoa/ScrollController.h: Added member to track stateless scrolling.
* platform/cocoa/ScrollController.mm: Added constant for delay for stateless scroll snapping.
(WebCore::ScrollController::processWheelEventForScrollSnapOnAxis): Triggers timer upon stateless scroll events.
(WebCore::ScrollController::horizontalScrollSnapTimerFired): Timer now handles stateless scrolling updates.
(WebCore::ScrollController::verticalScrollSnapTimerFired): See above.
(WebCore::ScrollController::beginScrollSnapAnimation): Handles stateless scroll snapping.

LayoutTests:

* platform/mac-wk2/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-overflow-stateless-expected.txt: Added.
* platform/mac-wk2/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-overflow-stateless.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-overflow-stateless-expected.txt [new file with mode: 0644]
LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-overflow-stateless.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/platform/cocoa/ScrollController.h
Source/WebCore/platform/cocoa/ScrollController.mm

index 844fa62..3e67a7f 100644 (file)
@@ -1,3 +1,14 @@
+2015-06-30  Wenson Hsieh  <whsieh@berkeley.edu>
+
+        scroll-snap-points do not work very well with mechanical scroll wheel events
+        https://bugs.webkit.org/show_bug.cgi?id=142501
+        <rdar://problem/20093511>
+
+        Reviewed by Brent Fulgham.
+
+        * platform/mac-wk2/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-overflow-stateless-expected.txt: Added.
+        * platform/mac-wk2/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-overflow-stateless.html: Added.
+
 2015-06-30  Chris Dumez  <cdumez@apple.com>
 
         Unreviewed, rebaseline editing/style/unbold-in-bold.html on WK2.
diff --git a/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-overflow-stateless-expected.txt b/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-overflow-stateless-expected.txt
new file mode 100644 (file)
index 0000000..3621139
--- /dev/null
@@ -0,0 +1,9 @@
+Tests that the scroll-snap feature works properly in overflow regions when using a mechanical mouse wheel.
+ PASS successfullyParsed is true
+
+TEST COMPLETE
+PASS div honored snap points.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
diff --git a/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-overflow-stateless.html b/LayoutTests/platform/mac-wk2/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-overflow-stateless.html
new file mode 100644 (file)
index 0000000..fad5f1c
--- /dev/null
@@ -0,0 +1,107 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <style>
+            .gallery {
+                width: 400px;
+                height: 400px;
+                display: inline-block;
+                overflow-x: hidden;
+                overflow-y: auto;
+                -webkit-scroll-snap-points-y: repeat(400px);
+                -webkit-scroll-snap-type: mandatory;
+            }
+            .colorBox {
+                height: 400px;
+                width: 400px;
+            }
+            #item0, #item2 { background-color: red; }
+            #item1 { background-color: green; }
+        </style>
+        <script src="../../../../../resources/js-test-pre.js"></script>
+        <script>
+        function finishTest()
+        {
+            finishJSTest();
+            testRunner.notifyDone();
+        }
+
+        function locationInWindowCoordinates(element)
+        {
+            var position = {};
+            position.x = element.offsetLeft;
+            position.y = element.offsetTop;
+
+            while (element.offsetParent) {
+                position.x = position.x + element.offsetParent.offsetLeft;
+                position.y = position.y + element.offsetParent.offsetTop;
+                if (element == document.getElementsByTagName("body")[0])
+                    break;
+
+                element = element.offsetParent;
+            }
+
+            return position;
+        }
+
+        function scrollSnapTest()
+        {
+            var divTarget = document.getElementById("target");
+            var windowPosition = locationInWindowCoordinates(divTarget);
+
+            var startPosX = windowPosition.x + 0.5 * divTarget.clientWidth;
+            var startPosY = windowPosition.y + 0.5 * divTarget.clientHeight;
+            eventSender.mouseMoveTo(startPosX, startPosY);
+            // Each unit of stateless scroll is scaled by 10 pixels.
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -10, "none", "none", false);
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -10, "none", "none", false);
+            eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -10, "none", "none", false);
+            // Wait for the snapping to finish.
+            setTimeout(function() {
+                if (divTarget.scrollTop != 400)
+                    testFailed("div did not snap to the green region. Expected 400, but got " + divTarget.scrollTop + ".");
+                else
+                    testPassed("div honored snap points.");
+
+                finishTest();
+            }, 2000);
+        }
+
+
+        function onLoad()
+        {
+            if (window.eventSender) {
+                window.jsTestIsAsync = true;
+                testRunner.dumpAsText();
+                testRunner.waitUntilDone();
+
+                eventSender.monitorWheelEvents();
+                setTimeout(scrollSnapTest, 0);
+            } else {
+                var messageLocationH = document.getElementById("item0");
+                var message = document.createElement("div");
+                message.innerHTML = "This test is better run under DumpRenderTree. To manually<br/>"
+                    + "test it, place the mouse pointer inside the red region at the<br/>"
+                    + "top of the page, and then, using a mechanical mouse wheel,<br/>"
+                    + "scroll so that the green region takes up more of the<br/>"
+                    + "container than the surrounding red regions. After a brief<br/>"
+                    + "pause, the target should scroll so that the green region is<br/>"
+                    + "fully visible, and no red is visible.";
+                messageLocationH.appendChild(message);
+            }
+        }
+        </script>
+    </head>
+    <body onload="onLoad();">
+        <div style="position: relative; width: 400px">
+            <div>Tests that the scroll-snap feature works properly in overflow regions when using a mechanical mouse wheel.</div>
+            <div class="gallery" id="target">
+                <div id="item0" class="colorBox"></div>
+                <div id="item1" class="colorBox"></div>
+                <div id="item2" class="colorBox"></div>
+            </div>
+            <div id="console"></div>
+        </div>
+        <script src="../../../../../resources/js-test-post.js"></script>
+    </body>
+</html>
\ No newline at end of file
index 2d98977..985045b 100644 (file)
@@ -1,3 +1,23 @@
+2015-06-30  Wenson Hsieh  <whsieh@berkeley.edu>
+
+        scroll-snap-points do not work very well with mechanical scroll wheel events
+        https://bugs.webkit.org/show_bug.cgi?id=142501
+        <rdar://problem/20093511>
+
+        Reviewed by Brent Fulgham.
+
+        Stateless scroll events generated by scrolling with a mechanical mouse wheel now trigger scroll
+        snapping after a fixed delay.
+
+        Test: platform/mac-wk2/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-overflow-stateless.html
+
+        * platform/cocoa/ScrollController.h: Added member to track stateless scrolling.
+        * platform/cocoa/ScrollController.mm: Added constant for delay for stateless scroll snapping.
+        (WebCore::ScrollController::processWheelEventForScrollSnapOnAxis): Triggers timer upon stateless scroll events.
+        (WebCore::ScrollController::horizontalScrollSnapTimerFired): Timer now handles stateless scrolling updates.
+        (WebCore::ScrollController::verticalScrollSnapTimerFired): See above.
+        (WebCore::ScrollController::beginScrollSnapAnimation): Handles stateless scroll snapping.
+
 2015-06-30  Simon Fraser  <simon.fraser@apple.com>
 
         Try to fix Gtk and EFL builds.
index 92cf12e..e896633 100644 (file)
@@ -188,6 +188,7 @@ private:
 #endif
 
 #if ENABLE(CSS_SCROLL_SNAP)
+    bool m_expectingStatelessScrollSnap { false };
     std::unique_ptr<ScrollSnapAnimatorState> m_horizontalScrollSnapState;
     std::unique_ptr<ScrollSnapAnimatorState> m_verticalScrollSnapState;
 #if PLATFORM(MAC)
index 048adb9..4a11fd5 100644 (file)
@@ -86,6 +86,8 @@ static const float glideBoostMultiplier = 3.5;
 
 static const float maxTargetWheelDelta = 7;
 static const float minTargetWheelDelta = 3.5;
+
+static const double statelessScrollSnapDelay = 0.5;
 #endif
 
 #if PLATFORM(MAC)
@@ -583,6 +585,11 @@ void ScrollController::processWheelEventForScrollSnapOnAxis(ScrollEventAxis axis
         endScrollSnapAnimation(axis, ScrollSnapState::UserInteraction);
         snapState.clearInitialWheelDeltaWindow();
         snapState.m_shouldOverrideWheelEvent = false;
+        m_expectingStatelessScrollSnap = true;
+        if (axis == ScrollEventAxis::Vertical)
+            m_verticalScrollSnapTimer.startOneShot(statelessScrollSnapDelay);
+        else
+            m_horizontalScrollSnapTimer.startOneShot(statelessScrollSnapDelay);
         break;
 
     case WheelEventStatus::Unknown:
@@ -668,12 +675,18 @@ void ScrollController::stopScrollSnapTimer(ScrollEventAxis axis)
 
 void ScrollController::horizontalScrollSnapTimerFired()
 {
-    scrollSnapAnimationUpdate(ScrollEventAxis::Horizontal);
+    if (m_expectingStatelessScrollSnap)
+        beginScrollSnapAnimation(ScrollEventAxis::Horizontal, ScrollSnapState::Snapping);
+    else
+        scrollSnapAnimationUpdate(ScrollEventAxis::Horizontal);
 }
 
 void ScrollController::verticalScrollSnapTimerFired()
 {
-    scrollSnapAnimationUpdate(ScrollEventAxis::Vertical);
+    if (m_expectingStatelessScrollSnap)
+        beginScrollSnapAnimation(ScrollEventAxis::Vertical, ScrollSnapState::Snapping);
+    else
+        scrollSnapAnimationUpdate(ScrollEventAxis::Vertical);
 }
 
 void ScrollController::scrollSnapAnimationUpdate(ScrollEventAxis axis)
@@ -768,7 +781,10 @@ void ScrollController::setActiveScrollSnapIndicesForOffset(int x, int y)
 void ScrollController::beginScrollSnapAnimation(ScrollEventAxis axis, ScrollSnapState newState)
 {
     ASSERT(newState == ScrollSnapState::Gliding || newState == ScrollSnapState::Snapping);
-    
+    if (m_expectingStatelessScrollSnap) {
+        m_expectingStatelessScrollSnap = false;
+        stopScrollSnapTimer(axis);
+    }
     ScrollSnapAnimatorState& snapState = scrollSnapPointState(axis);
 
     LayoutUnit offset = m_client.scrollOffsetOnAxis(axis);