- Revision
- 209477
- Author
- wenson_hs...@apple.com
- Date
- 2016-12-07 13:50:11 -0800 (Wed, 07 Dec 2016)
Log Message
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.
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (209476 => 209477)
--- trunk/LayoutTests/ChangeLog 2016-12-07 21:43:58 UTC (rev 209476)
+++ trunk/LayoutTests/ChangeLog 2016-12-07 21:50:11 UTC (rev 209477)
@@ -1,3 +1,17 @@
+2016-12-07 Wenson Hsieh <wenson_hs...@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.fra...@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
Added: trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-scrolling-jumps-to-top.html (0 => 209477)
--- trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-scrolling-jumps-to-top.html (rev 0)
+++ trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-scrolling-jumps-to-top.html 2016-12-07 21:50:11 UTC (rev 209477)
@@ -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>
Added: trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-scrolling-jumps-to-top.txt (0 => 209477)
--- trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-scrolling-jumps-to-top.txt (rev 0)
+++ trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-scrolling-jumps-to-top.txt 2016-12-07 21:50:11 UTC (rev 209477)
@@ -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
+
Modified: trunk/Source/WebCore/ChangeLog (209476 => 209477)
--- trunk/Source/WebCore/ChangeLog 2016-12-07 21:43:58 UTC (rev 209476)
+++ trunk/Source/WebCore/ChangeLog 2016-12-07 21:50:11 UTC (rev 209477)
@@ -1,3 +1,25 @@
+2016-12-07 Wenson Hsieh <wenson_hs...@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_w...@apple.com>
AX: menu type toolbar should be mapped correctly on Mac
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingMomentumCalculatorMac.h (209476 => 209477)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingMomentumCalculatorMac.h 2016-12-07 21:43:58 UTC (rev 209476)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingMomentumCalculatorMac.h 2016-12-07 21:50:11 UTC (rev 209477)
@@ -44,6 +44,7 @@
_NSScrollingMomentumCalculator *ensurePlatformMomentumCalculator();
RetainPtr<_NSScrollingMomentumCalculator> m_platformMomentumCalculator;
+ bool m_requiresMomentumScrolling { true };
};
} // namespace WebCore
Modified: trunk/Source/WebCore/page/scrolling/mac/ScrollingMomentumCalculatorMac.mm (209476 => 209477)
--- trunk/Source/WebCore/page/scrolling/mac/ScrollingMomentumCalculatorMac.mm 2016-12-07 21:43:58 UTC (rev 209476)
+++ trunk/Source/WebCore/page/scrolling/mac/ScrollingMomentumCalculatorMac.mm 2016-12-07 21:50:11 UTC (rev 209477)
@@ -39,16 +39,23 @@
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]);
}
