Scroll position jumps to the origin when scrolling without momentum at the end of...
authorwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Dec 2016 21:50:11 +0000 (21:50 +0000)
committerwenson_hsieh@apple.com <wenson_hsieh@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 7 Dec 2016 21:50:11 +0000 (21:50 +0000)
https://bugs.webkit.org/show_bug.cgi?id=165474
<rdar://problem/29534305>

Reviewed by Simon Fraser.

Source/WebCore:

When initializing an AppKit _NSScrollingMomentumCalculator, if the initial and target positions are the same and
the initial velocity is (0, 0), the momentum calculator will output (0, 0) as the animated scroll position when
animating. This causes the scroll position to jump to the top left in some cases when scrolling in scroll snap
containers. To fix this, we teach the ScrollingMomentumCalculatorMac to return an animation duration of 0 and
an animated scroll position equal to the final scroll position when this is the case.

Test: tiled-drawing/scrolling/scroll-snap/scrolling-jumps-to-top.html

* page/scrolling/mac/ScrollingMomentumCalculatorMac.h:
* page/scrolling/mac/ScrollingMomentumCalculatorMac.mm:
(WebCore::ScrollingMomentumCalculatorMac::ScrollingMomentumCalculatorMac):
(WebCore::ScrollingMomentumCalculatorMac::scrollOffsetAfterElapsedTime):
(WebCore::ScrollingMomentumCalculatorMac::animationDuration):

LayoutTests:

Added a new test verifying that if a scroll gesture ends without momentum at the bottom of a scroll snapping
container, the scroll position won't jump to the top.

* tiled-drawing/scrolling/scroll-snap/scrolling-jumps-to-top-expected.txt: Added.
* tiled-drawing/scrolling/scroll-snap/scrolling-jumps-to-top.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-scrolling-jumps-to-top.html [new file with mode: 0644]
LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-scrolling-jumps-to-top.txt [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/mac/ScrollingMomentumCalculatorMac.h
Source/WebCore/page/scrolling/mac/ScrollingMomentumCalculatorMac.mm

index 48d35be..421d789 100644 (file)
@@ -1,3 +1,17 @@
+2016-12-07  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Scroll position jumps to the origin when scrolling without momentum at the end of a scroll snapping container
+        https://bugs.webkit.org/show_bug.cgi?id=165474
+        <rdar://problem/29534305>
+
+        Reviewed by Simon Fraser.
+
+        Added a new test verifying that if a scroll gesture ends without momentum at the bottom of a scroll snapping
+        container, the scroll position won't jump to the top.
+
+        * tiled-drawing/scrolling/scroll-snap/scrolling-jumps-to-top-expected.txt: Added.
+        * tiled-drawing/scrolling/scroll-snap/scrolling-jumps-to-top.html: Added.
+
 2016-12-07  Simon Fraser  <simon.fraser@apple.com>
 
         REGRESSION (r209447): LayoutTests compositing/layer-creation/fixed-position-out-of-view-scaled.html and compositing/layer-creation/fixed-position-out-of-view-scaled-scroll.html failing
diff --git a/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-scrolling-jumps-to-top.html b/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-scrolling-jumps-to-top.html
new file mode 100644 (file)
index 0000000..0677f0d
--- /dev/null
@@ -0,0 +1,68 @@
+<!DOCTYPE html>
+<html>
+    <head>
+        <style>
+            body {
+                overflow: hidden;
+                margin: 0;
+            }
+
+            #container {
+                width: 300px;
+                height: 300px;
+                display: inline-block;
+                overflow-x: hidden;
+                overflow-y: auto;
+                scroll-snap-type: y mandatory;
+                -webkit-scroll-snap-type: mandatory;
+            }
+
+            .child {
+                height: 300px;
+                width: 300px;
+                float: left;
+                scroll-snap-align: start;
+                -webkit-scroll-snap-coordinate: 0 0;
+            }
+        </style>
+        <script>
+        let write = s => output.innerHTML += s + "<br>";
+        function run() {
+            container.scrollTop = 300;
+            write(`The scroll position is: ${container.scrollTop}`);
+            if (!window.eventSender || !window.testRunner)
+                return;
+
+            testRunner.dumpAsText();
+            testRunner.waitUntilDone();
+            eventSender.monitorWheelEvents();
+            eventSender.mouseMoveTo(container.offsetLeft + container.clientWidth / 2, container.offsetTop + container.clientHeight / 2);
+
+            write("Scrolling without momentum to the same position several times")
+            for (let i = 0; i < 10; i++) {
+                eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "began", "none");
+                eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 1, "changed", "none");
+                eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "changed", "none");
+                eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "changed", "none");
+                eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "ended", "none");
+            }
+
+            setTimeout(() => {
+                eventSender.callAfterScrollingCompletes(() => {
+                    write(`The scroll position is now: ${container.scrollTop}`);
+                    testRunner.notifyDone();
+                });
+            }, 0);
+        }
+        </script>
+    </head>
+    <body onload=run()>
+        <p>To manually test, scroll so the green box exactly fills the container, and let go.</p>
+        <p>The scroll position should not jump to the top.</p>
+        <div id="container">
+            <div style="background-color: red;" class="child"></div>
+            <div style="background-color: green;" class="child"></div>
+        </div>
+        <div id="output"></div>
+    </body>
+</html>
diff --git a/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-scrolling-jumps-to-top.txt b/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-scrolling-jumps-to-top.txt
new file mode 100644 (file)
index 0000000..71af194
--- /dev/null
@@ -0,0 +1,8 @@
+To manually test, scroll so the green box exactly fills the container, and let go.
+
+The scroll position should not jump to the top.
+
+The scroll position is: 300
+Scrolling without momentum to the same position several times
+The scroll position is now: 300
+
index bb986c6..3a97bb0 100644 (file)
@@ -1,3 +1,25 @@
+2016-12-07  Wenson Hsieh  <wenson_hsieh@apple.com>
+
+        Scroll position jumps to the origin when scrolling without momentum at the end of a scroll snapping container
+        https://bugs.webkit.org/show_bug.cgi?id=165474
+        <rdar://problem/29534305>
+
+        Reviewed by Simon Fraser.
+
+        When initializing an AppKit _NSScrollingMomentumCalculator, if the initial and target positions are the same and
+        the initial velocity is (0, 0), the momentum calculator will output (0, 0) as the animated scroll position when
+        animating. This causes the scroll position to jump to the top left in some cases when scrolling in scroll snap
+        containers. To fix this, we teach the ScrollingMomentumCalculatorMac to return an animation duration of 0 and
+        an animated scroll position equal to the final scroll position when this is the case.
+
+        Test: tiled-drawing/scrolling/scroll-snap/scrolling-jumps-to-top.html
+
+        * page/scrolling/mac/ScrollingMomentumCalculatorMac.h:
+        * page/scrolling/mac/ScrollingMomentumCalculatorMac.mm:
+        (WebCore::ScrollingMomentumCalculatorMac::ScrollingMomentumCalculatorMac):
+        (WebCore::ScrollingMomentumCalculatorMac::scrollOffsetAfterElapsedTime):
+        (WebCore::ScrollingMomentumCalculatorMac::animationDuration):
+
 2016-12-07  Nan Wang  <n_wang@apple.com>
 
         AX: menu type toolbar should be mapped correctly on Mac
index b6cc4bf..456a113 100644 (file)
@@ -44,6 +44,7 @@ private:
     _NSScrollingMomentumCalculator *ensurePlatformMomentumCalculator();
 
     RetainPtr<_NSScrollingMomentumCalculator> m_platformMomentumCalculator;
+    bool m_requiresMomentumScrolling { true };
 };
 
 } // namespace WebCore
index 19ae1c9..f72455e 100644 (file)
@@ -39,16 +39,23 @@ std::unique_ptr<ScrollingMomentumCalculator> ScrollingMomentumCalculator::create
 
 ScrollingMomentumCalculatorMac::ScrollingMomentumCalculatorMac(const FloatSize& viewportSize, const FloatSize& contentSize, const FloatPoint& initialOffset, const FloatPoint& targetOffset, const FloatSize& initialDelta, const FloatSize& initialVelocity)
     : ScrollingMomentumCalculator(viewportSize, contentSize, initialOffset, targetOffset, initialDelta, initialVelocity)
+    , m_requiresMomentumScrolling(initialOffset != targetOffset || initialVelocity.area())
 {
 }
 
 FloatPoint ScrollingMomentumCalculatorMac::scrollOffsetAfterElapsedTime(Seconds elapsedTime)
 {
+    if (!m_requiresMomentumScrolling)
+        return { m_targetScrollOffset.width(), m_targetScrollOffset.height() };
+
     return [ensurePlatformMomentumCalculator() positionAfterDuration:elapsedTime.value()];
 }
 
 Seconds ScrollingMomentumCalculatorMac::animationDuration()
 {
+    if (!m_requiresMomentumScrolling)
+        return 0_s;
+
     return Seconds([ensurePlatformMomentumCalculator() durationUntilStop]);
 }