<!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>[210117] 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/210117">210117</a></dd>
<dt>Author</dt> <dd>wenson_hsieh@apple.com</dd>
<dt>Date</dt> <dd>2016-12-22 14:55:31 -0800 (Thu, 22 Dec 2016)</dd>
</dl>
<h3>Log Message</h3>
<pre>CSS Scroll Snap does not work if scrollbar is hidden
https://bugs.webkit.org/show_bug.cgi?id=160442
<rdar://problem/23317034>
Reviewed by Simon Fraser.
Source/WebCore:
Currently, the only reason scroll snapping works in overflow scrolling containers without forcing layout is
because we would initialize the scrolling container's ScrollAnimator in the process of updating scrollbars. If
there are no scrollbars to render, we won't bother creating a ScrollAnimator. Without an existing
ScrollAnimator, ScrollableArea::updateScrollSnapState will simply bail instead of setting up the scroll snap
state. Instead, we should take setting a non-empty vector of scroll offsets on the ScrollableArea as a cue that
the ScrollableArea also needs a ScrollAnimator, and initialize it there if necessary.
Test: tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars.html
* platform/ScrollableArea.cpp:
(WebCore::ScrollableArea::setHorizontalSnapOffsets):
(WebCore::ScrollableArea::setVerticalSnapOffsets):
LayoutTests:
Adds a new layout test verifying that scroll snapping still works when scrollbars are hidden via CSS.
* tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars-expected.txt: Added.
* tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars.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="#trunkSourceWebCoreplatformScrollableAreacpp">trunk/Source/WebCore/platform/ScrollableArea.cpp</a></li>
</ul>
<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTeststileddrawingscrollingscrollsnapscrollsnapmandatoryhiddenscrollbarsexpectedtxt">trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars-expected.txt</a></li>
<li><a href="#trunkLayoutTeststileddrawingscrollingscrollsnapscrollsnapmandatoryhiddenscrollbarshtml">trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars.html</a></li>
</ul>
</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (210116 => 210117)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog 2016-12-22 22:48:32 UTC (rev 210116)
+++ trunk/LayoutTests/ChangeLog 2016-12-22 22:55:31 UTC (rev 210117)
</span><span class="lines">@@ -1,3 +1,16 @@
</span><ins>+2016-12-22 Wenson Hsieh <wenson_hsieh@apple.com>
+
+ CSS Scroll Snap does not work if scrollbar is hidden
+ https://bugs.webkit.org/show_bug.cgi?id=160442
+ <rdar://problem/23317034>
+
+ Reviewed by Simon Fraser.
+
+ Adds a new layout test verifying that scroll snapping still works when scrollbars are hidden via CSS.
+
+ * tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars-expected.txt: Added.
+ * tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars.html: Added.
+
</ins><span class="cx"> 2016-12-22 Daniel Bates <dabates@apple.com>
</span><span class="cx">
</span><span class="cx"> Make http/tests/security/popup-blocked-from-{fake-event, window-open}.html actually test popup
</span></span></pre></div>
<a id="trunkLayoutTeststileddrawingscrollingscrollsnapscrollsnapmandatoryhiddenscrollbarsexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars-expected.txt (0 => 210117)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars-expected.txt (rev 0)
+++ trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars-expected.txt 2016-12-22 22:55:31 UTC (rev 210117)
</span><span class="lines">@@ -0,0 +1,2 @@
</span><ins>+After swiping, the container's scrollTop is now: 500
+
</ins></span></pre></div>
<a id="trunkLayoutTeststileddrawingscrollingscrollsnapscrollsnapmandatoryhiddenscrollbarshtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars.html (0 => 210117)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars.html (rev 0)
+++ trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars.html 2016-12-22 22:55:31 UTC (rev 210117)
</span><span class="lines">@@ -0,0 +1,74 @@
</span><ins>+<!DOCTYPE HTML>
+<html>
+ <head>
+ <style>
+ body {
+ margin: 0;
+ overflow: hidden;
+ }
+
+ ::-webkit-scrollbar {
+ display: none;
+ }
+
+ #port {
+ width: 500px;
+ height: 500px;
+ position: absolute;
+ top: 0;
+ left: 0;
+ overflow-x: none;
+ overflow-y: scroll;
+ scroll-snap-type: y mandatory;
+ opacity: 0.5;
+ }
+
+ .area {
+ height: 500px;
+ width: 500px;
+ float: left;
+ scroll-snap-align: center;
+ }
+ </style>
+ <script>
+ let write = s => output.innerHTML += s + "<br>";
+
+ if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+ }
+
+ function run() {
+ if (!window.testRunner || !window.eventSender) {
+ write("To manually test, verify that scrolling in the overflow container snaps to each of the children.");
+ return;
+ }
+
+ eventSender.monitorWheelEvents();
+ eventSender.mouseMoveTo(250, 250);
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "began", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "changed", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "changed", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "ended", "none");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "none", "begin");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, "none", "continue");
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, "none", "end");
+ eventSender.callAfterScrollingCompletes(() => {
+ write(`After swiping, the container's scrollTop is now: ${port.scrollTop}`);
+ testRunner.notifyDone();
+ });
+ }
+ </script>
+ </head>
+ <body onload=run()>
+ <div id="port">
+ <div class="area" style="background-color: red;"></div>
+ <div class="area" style="background-color: green;"></div>
+ <div class="area" style="background-color: blue;"></div>
+ <div class="area" style="background-color: aqua;"></div>
+ <div class="area" style="background-color: yellow;"></div>
+ <div class="area" style="background-color: fuchsia;"></div>
+ </div>
+ <div id="output"></div>
+ </body>
+</html>
</ins></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (210116 => 210117)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog 2016-12-22 22:48:32 UTC (rev 210116)
+++ trunk/Source/WebCore/ChangeLog 2016-12-22 22:55:31 UTC (rev 210117)
</span><span class="lines">@@ -1,3 +1,24 @@
</span><ins>+2016-12-22 Wenson Hsieh <wenson_hsieh@apple.com>
+
+ CSS Scroll Snap does not work if scrollbar is hidden
+ https://bugs.webkit.org/show_bug.cgi?id=160442
+ <rdar://problem/23317034>
+
+ Reviewed by Simon Fraser.
+
+ Currently, the only reason scroll snapping works in overflow scrolling containers without forcing layout is
+ because we would initialize the scrolling container's ScrollAnimator in the process of updating scrollbars. If
+ there are no scrollbars to render, we won't bother creating a ScrollAnimator. Without an existing
+ ScrollAnimator, ScrollableArea::updateScrollSnapState will simply bail instead of setting up the scroll snap
+ state. Instead, we should take setting a non-empty vector of scroll offsets on the ScrollableArea as a cue that
+ the ScrollableArea also needs a ScrollAnimator, and initialize it there if necessary.
+
+ Test: tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-hidden-scrollbars.html
+
+ * platform/ScrollableArea.cpp:
+ (WebCore::ScrollableArea::setHorizontalSnapOffsets):
+ (WebCore::ScrollableArea::setVerticalSnapOffsets):
+
</ins><span class="cx"> 2016-12-22 Daniel Bates <dabates@apple.com>
</span><span class="cx">
</span><span class="cx"> Bypass pop-up blocker from cross-origin or sandboxed frame
</span></span></pre></div>
<a id="trunkSourceWebCoreplatformScrollableAreacpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/platform/ScrollableArea.cpp (210116 => 210117)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/platform/ScrollableArea.cpp 2016-12-22 22:48:32 UTC (rev 210116)
+++ trunk/Source/WebCore/platform/ScrollableArea.cpp 2016-12-22 22:55:31 UTC (rev 210117)
</span><span class="lines">@@ -427,11 +427,21 @@
</span><span class="cx"> #if ENABLE(CSS_SCROLL_SNAP)
</span><span class="cx"> void ScrollableArea::setHorizontalSnapOffsets(std::unique_ptr<Vector<LayoutUnit>> horizontalSnapOffsets)
</span><span class="cx"> {
</span><ins>+ ASSERT(horizontalSnapOffsets);
+ // Consider having a non-empty set of snap offsets as a cue to initialize the ScrollAnimator.
+ if (horizontalSnapOffsets->size())
+ scrollAnimator();
+
</ins><span class="cx"> m_horizontalSnapOffsets = WTFMove(horizontalSnapOffsets);
</span><span class="cx"> }
</span><span class="cx">
</span><span class="cx"> void ScrollableArea::setVerticalSnapOffsets(std::unique_ptr<Vector<LayoutUnit>> verticalSnapOffsets)
</span><span class="cx"> {
</span><ins>+ ASSERT(verticalSnapOffsets);
+ // Consider having a non-empty set of snap offsets as a cue to initialize the ScrollAnimator.
+ if (verticalSnapOffsets->size())
+ scrollAnimator();
+
</ins><span class="cx"> m_verticalSnapOffsets = WTFMove(verticalSnapOffsets);
</span><span class="cx"> }
</span><span class="cx">
</span></span></pre>
</div>
</div>
</body>
</html>