<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[215075] trunk</title>
</head>
<body>
<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; }
#msg dl a { font-weight: bold}
#msg dl a:link { color:#fc3; }
#msg dl a:active { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/215075">215075</a></dd>
<dt>Author</dt> <dd>wenson_hsieh@apple.com</dd>
<dt>Date</dt> <dd>2017-04-06 17:38:42 -0700 (Thu, 06 Apr 2017)</dd>
</dl>
<h3>Log Message</h3>
<pre>Scroll offset jumps after a programmatic scroll in an overflow container with scroll snapping
https://bugs.webkit.org/show_bug.cgi?id=170560
<rdar://problem/31484693>
Reviewed by Tim Horton.
Source/WebCore:
Test: css3/scroll-snap/scroll-snap-programmatic-overflow-scroll.html
Logic for maintaining the scroll snap state in ScrollController was previously removed from iOS when refactoring
ScrollController. This was done because scroll snapping on iOS is driven not by the ScrollController (as it is
on Mac) but rather by sending scroll snap offsets to the UI process and hooking into UIScrollView delegates to
handle retargeted scrolling.
However, on iOS, this ScrollController state is still important for the purposes of keeping the last active
snap point index in sync with the UI process when the scroll offset changes outside of a user gesture (i.e.
programmatic scrolling). Since the UI process does not get a chance to update the active snap offset during a
programmatic scroll, our last active snap offset state was only being updated to the last snap position that the
user manually scrolled to, making programmatic scrolling jump to this offset.
To fix this, we need to update scroll snap state on iOS within ScrollController. Also adds a new Layout test
that exercises programmatic scrolling in an overflow scrolling container on all platforms.
* platform/cocoa/ScrollController.mm:
(WebCore::otherScrollEventAxis):
(WebCore::ScrollController::updateScrollSnapState):
(WebCore::ScrollController::updateScrollSnapPoints):
LayoutTests:
Add a test verifying that programmatically changing the scroll offset of an overflow container does not cause the
scroll offset to jump back to the last active snap position. See WebCore ChangeLog for more details.
* css3/scroll-snap/scroll-snap-programmatic-overflow-scroll-expected.txt: Added.
* css3/scroll-snap/scroll-snap-programmatic-overflow-scroll.html: Added.</pre>
<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCoreplatformcocoaScrollControllermm">trunk/Source/WebCore/platform/cocoa/ScrollController.mm</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestscss3scrollsnapscrollsnapprogrammaticoverflowscrollexpectedtxt">trunk/LayoutTests/css3/scroll-snap/scroll-snap-programmatic-overflow-scroll-expected.txt</a></li>
<li><a href="#trunkLayoutTestscss3scrollsnapscrollsnapprogrammaticoverflowscrollhtml">trunk/LayoutTests/css3/scroll-snap/scroll-snap-programmatic-overflow-scroll.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (215074 => 215075)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2017-04-07 00:36:53 UTC (rev 215074)
+++ trunk/LayoutTests/ChangeLog 2017-04-07 00:38:42 UTC (rev 215075)
</span><span class="lines">@@ -1,3 +1,17 @@
</span><ins>+2017-04-06 Wenson Hsieh <wenson_hsieh@apple.com>
+
+ Scroll offset jumps after a programmatic scroll in an overflow container with scroll snapping
+ https://bugs.webkit.org/show_bug.cgi?id=170560
+ <rdar://problem/31484693>
+
+ Reviewed by Tim Horton.
+
+ Add a test verifying that programmatically changing the scroll offset of an overflow container does not cause the
+ scroll offset to jump back to the last active snap position. See WebCore ChangeLog for more details.
+
+ * css3/scroll-snap/scroll-snap-programmatic-overflow-scroll-expected.txt: Added.
+ * css3/scroll-snap/scroll-snap-programmatic-overflow-scroll.html: Added.
+
</ins><span class="cx"> 2017-04-05 Simon Fraser <simon.fraser@apple.com>
</span><span class="cx">
</span><span class="cx"> Throttle requestAnimationFrame in cross-origin iframes to 30fps
</span></span></pre></div>
<a id="trunkLayoutTestscss3scrollsnapscrollsnapprogrammaticoverflowscrollexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/css3/scroll-snap/scroll-snap-programmatic-overflow-scroll-expected.txt (0 => 215075)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/css3/scroll-snap/scroll-snap-programmatic-overflow-scroll-expected.txt (rev 0)
+++ trunk/LayoutTests/css3/scroll-snap/scroll-snap-programmatic-overflow-scroll-expected.txt 2017-04-07 00:38:42 UTC (rev 215075)
</span><span class="lines">@@ -0,0 +1 @@
</span><ins>+container.scrollTop is now: 400
</ins></span></pre></div>
<a id="trunkLayoutTestscss3scrollsnapscrollsnapprogrammaticoverflowscrollhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/css3/scroll-snap/scroll-snap-programmatic-overflow-scroll.html (0 => 215075)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/css3/scroll-snap/scroll-snap-programmatic-overflow-scroll.html (rev 0)
+++ trunk/LayoutTests/css3/scroll-snap/scroll-snap-programmatic-overflow-scroll.html 2017-04-07 00:38:42 UTC (rev 215075)
</span><span class="lines">@@ -0,0 +1,52 @@
</span><ins>+<head>
+ <meta name="viewport" content="width=device-width, initial-scale=1">
+ <style>
+ html, body {
+ overflow: hidden;
+ padding: 0;
+ margin: 0;
+ }
+
+ #container {
+ width: 100%;
+ height: 400px;
+ overflow-y: auto;
+ position: absolute;
+ -webkit-overflow-scrolling: touch;
+ scroll-snap-type: y mandatory;
+ }
+
+ .child {
+ width: 100%;
+ height: 100%;
+ scroll-snap-align: start;
+ }
+
+ #fixed {
+ position: fixed;
+ top: 0;
+ right: 0;
+ width: 1em;
+ height: 1em;
+ background-color: white;
+ }
+
+ #output {
+ margin-top: 400px;
+ }
+ </style>
+</head>
+<div id="container">
+ <div class="child" style="background-color: red"></div>
+ <div class="child" style="background-color: green"></div>
+ <div class="child" style="background-color: blue"></div>
+</div>
+<div id="fixed"></div>
+<script>
+if (window.testRunner)
+ testRunner.dumpAsText();
+
+container.scrollTop = 400;
+fixed.remove();
+document.write(`container.scrollTop is now: ${container.scrollTop}`);
+</script>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (215074 => 215075)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2017-04-07 00:36:53 UTC (rev 215074)
+++ trunk/Source/WebCore/ChangeLog 2017-04-07 00:38:42 UTC (rev 215075)
</span><span class="lines">@@ -1,3 +1,32 @@
</span><ins>+2017-04-06 Wenson Hsieh <wenson_hsieh@apple.com>
+
+ Scroll offset jumps after a programmatic scroll in an overflow container with scroll snapping
+ https://bugs.webkit.org/show_bug.cgi?id=170560
+ <rdar://problem/31484693>
+
+ Reviewed by Tim Horton.
+
+ Test: css3/scroll-snap/scroll-snap-programmatic-overflow-scroll.html
+
+ Logic for maintaining the scroll snap state in ScrollController was previously removed from iOS when refactoring
+ ScrollController. This was done because scroll snapping on iOS is driven not by the ScrollController (as it is
+ on Mac) but rather by sending scroll snap offsets to the UI process and hooking into UIScrollView delegates to
+ handle retargeted scrolling.
+
+ However, on iOS, this ScrollController state is still important for the purposes of keeping the last active
+ snap point index in sync with the UI process when the scroll offset changes outside of a user gesture (i.e.
+ programmatic scrolling). Since the UI process does not get a chance to update the active snap offset during a
+ programmatic scroll, our last active snap offset state was only being updated to the last snap position that the
+ user manually scrolled to, making programmatic scrolling jump to this offset.
+
+ To fix this, we need to update scroll snap state on iOS within ScrollController. Also adds a new Layout test
+ that exercises programmatic scrolling in an overflow scrolling container on all platforms.
+
+ * platform/cocoa/ScrollController.mm:
+ (WebCore::otherScrollEventAxis):
+ (WebCore::ScrollController::updateScrollSnapState):
+ (WebCore::ScrollController::updateScrollSnapPoints):
+
</ins><span class="cx"> 2017-04-05 Simon Fraser <simon.fraser@apple.com>
</span><span class="cx">
</span><span class="cx"> Throttle requestAnimationFrame in cross-origin iframes to 30fps
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformcocoaScrollControllermm"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/cocoa/ScrollController.mm (215074 => 215075)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/cocoa/ScrollController.mm 2017-04-07 00:36:53 UTC (rev 215074)
+++ trunk/Source/WebCore/platform/cocoa/ScrollController.mm 2017-04-07 00:38:42 UTC (rev 215075)
</span><span class="lines">@@ -75,12 +75,12 @@
</span><span class="cx"> }
</span><span class="cx"> return multiplier;
</span><span class="cx"> }
</span><ins>+#endif
</ins><span class="cx">
</span><span class="cx"> static ScrollEventAxis otherScrollEventAxis(ScrollEventAxis axis)
</span><span class="cx"> {
</span><span class="cx"> return axis == ScrollEventAxis::Horizontal ? ScrollEventAxis::Vertical : ScrollEventAxis::Horizontal;
</span><span class="cx"> }
</span><del>-#endif
</del><span class="cx">
</span><span class="cx"> ScrollController::ScrollController(ScrollControllerClient& client)
</span><span class="cx"> : m_client(client)
</span><span class="lines">@@ -555,40 +555,6 @@
</span><span class="cx"> return !(isInertialScrolling && shouldOverrideInertialScrolling());
</span><span class="cx"> }
</span><span class="cx">
</span><del>-void ScrollController::updateScrollSnapState(const ScrollableArea& scrollableArea)
-{
- if (auto* snapOffsets = scrollableArea.horizontalSnapOffsets()) {
- if (auto* snapOffsetRanges = scrollableArea.horizontalSnapOffsetRanges())
- updateScrollSnapPoints(ScrollEventAxis::Horizontal, *snapOffsets, *snapOffsetRanges);
- else
- updateScrollSnapPoints(ScrollEventAxis::Horizontal, *snapOffsets, { });
- } else
- updateScrollSnapPoints(ScrollEventAxis::Horizontal, { }, { });
-
- if (auto* snapOffsets = scrollableArea.verticalSnapOffsets()) {
- if (auto* snapOffsetRanges = scrollableArea.verticalSnapOffsetRanges())
- updateScrollSnapPoints(ScrollEventAxis::Vertical, *snapOffsets, *snapOffsetRanges);
- else
- updateScrollSnapPoints(ScrollEventAxis::Vertical, *snapOffsets, { });
- } else
- updateScrollSnapPoints(ScrollEventAxis::Vertical, { }, { });
-}
-
-void ScrollController::updateScrollSnapPoints(ScrollEventAxis axis, const Vector<LayoutUnit>& snapPoints, const Vector<ScrollOffsetRange<LayoutUnit>>& snapRanges)
-{
- if (!m_scrollSnapState) {
- if (snapPoints.isEmpty())
- return;
-
- m_scrollSnapState = std::make_unique<ScrollSnapAnimatorState>();
- }
-
- if (snapPoints.isEmpty() && m_scrollSnapState->snapOffsetsForAxis(otherScrollEventAxis(axis)).isEmpty())
- m_scrollSnapState = nullptr;
- else
- m_scrollSnapState->setSnapOffsetsAndPositionRangesForAxis(axis, snapPoints, snapRanges);
-}
-
</del><span class="cx"> void ScrollController::startScrollSnapTimer()
</span><span class="cx"> {
</span><span class="cx"> if (m_scrollSnapTimer.isActive())
</span><span class="lines">@@ -625,12 +591,42 @@
</span><span class="cx"> stopScrollSnapTimer();
</span><span class="cx"> }
</span><span class="cx"> }
</span><del>-#else
-void ScrollController::updateScrollSnapState(const ScrollableArea&)
</del><ins>+#endif // PLATFORM(MAC)
+
+void ScrollController::updateScrollSnapState(const ScrollableArea& scrollableArea)
</ins><span class="cx"> {
</span><ins>+ if (auto* snapOffsets = scrollableArea.horizontalSnapOffsets()) {
+ if (auto* snapOffsetRanges = scrollableArea.horizontalSnapOffsetRanges())
+ updateScrollSnapPoints(ScrollEventAxis::Horizontal, *snapOffsets, *snapOffsetRanges);
+ else
+ updateScrollSnapPoints(ScrollEventAxis::Horizontal, *snapOffsets, { });
+ } else
+ updateScrollSnapPoints(ScrollEventAxis::Horizontal, { }, { });
+
+ if (auto* snapOffsets = scrollableArea.verticalSnapOffsets()) {
+ if (auto* snapOffsetRanges = scrollableArea.verticalSnapOffsetRanges())
+ updateScrollSnapPoints(ScrollEventAxis::Vertical, *snapOffsets, *snapOffsetRanges);
+ else
+ updateScrollSnapPoints(ScrollEventAxis::Vertical, *snapOffsets, { });
+ } else
+ updateScrollSnapPoints(ScrollEventAxis::Vertical, { }, { });
</ins><span class="cx"> }
</span><del>-#endif // PLATFORM(MAC)
</del><span class="cx">
</span><ins>+void ScrollController::updateScrollSnapPoints(ScrollEventAxis axis, const Vector<LayoutUnit>& snapPoints, const Vector<ScrollOffsetRange<LayoutUnit>>& snapRanges)
+{
+ if (!m_scrollSnapState) {
+ if (snapPoints.isEmpty())
+ return;
+
+ m_scrollSnapState = std::make_unique<ScrollSnapAnimatorState>();
+ }
+
+ if (snapPoints.isEmpty() && m_scrollSnapState->snapOffsetsForAxis(otherScrollEventAxis(axis)).isEmpty())
+ m_scrollSnapState = nullptr;
+ else
+ m_scrollSnapState->setSnapOffsetsAndPositionRangesForAxis(axis, snapPoints, snapRanges);
+}
+
</ins><span class="cx"> unsigned ScrollController::activeScrollSnapIndexForAxis(ScrollEventAxis axis) const
</span><span class="cx"> {
</span><span class="cx"> if (!m_scrollSnapState)
</span></span></pre>
</div>
</div>
</body>
</html>