Diff
Modified: trunk/LayoutTests/ChangeLog (269621 => 269622)
--- trunk/LayoutTests/ChangeLog 2020-11-10 09:05:17 UTC (rev 269621)
+++ trunk/LayoutTests/ChangeLog 2020-11-10 09:28:40 UTC (rev 269622)
@@ -1,3 +1,18 @@
+2020-11-10 Martin Robinson <mrobin...@igalia.com>
+
+ Scroll-snap on the root aligns to the body margin edge, not the viewport edge
+ https://bugs.webkit.org/show_bug.cgi?id=210476
+ <rdar://problem/61755103>
+
+ Reviewed by Simon Fraser.
+
+ Added tests for new behavior.
+
+ * tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-horizontal-with-margin-expected.txt: Added.
+ * tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-horizontal-with-margin.html: Added.
+ * tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-with-margin-expected.txt: Added.
+ * tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-with-margin.html: Added.
+
2020-11-09 Chris Dumez <cdu...@apple.com>
Unexpose obsolete HTMLAppletElement interface
Added: trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-horizontal-with-margin-expected.txt (0 => 269622)
--- trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-horizontal-with-margin-expected.txt (rev 0)
+++ trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-horizontal-with-margin-expected.txt 2020-11-10 09:28:40 UTC (rev 269622)
@@ -0,0 +1,6 @@
+PASS div scrolled to next window.
+PASS div honored snap points.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-horizontal-with-margin.html (0 => 269622)
--- trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-horizontal-with-margin.html (rev 0)
+++ trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-horizontal-with-margin.html 2020-11-10 09:28:40 UTC (rev 269622)
@@ -0,0 +1,120 @@
+<!DOCTYPE HTML>
+<html>
+ <head>
+ <style>
+ html {
+ scroll-snap-type: x mandatory;
+ margin-left: 50px;
+ /* There's a different value for the right margin to ensure that
+ margins with different sizes are properly tested. */
+ margin-right: 100px;
+ }
+ .horizontalGallery {
+ width: 600vw;
+ height: 100vh;
+ margin: 0;
+ padding: 0;
+ }
+ .colorBox {
+ height: 100vh;
+ width: 100vw;
+ float: left;
+ scroll-snap-align: start;
+ }
+ #item0 { background-color: red; }
+ #item1 { background-color: green; }
+ #item2 { background-color: blue; }
+ #item3 { background-color: aqua; }
+ #item4 { background-color: yellow; }
+ #item5 { background-color: fuchsia; }
+ </style>
+ <script src=""
+ <script>
+ window.jsTestIsAsync = true;
+
+ var targetScroller;
+ var scrollPositionBeforeGlide;
+ var scrollPositionBeforeSnap;
+
+ function checkForScrollSnap()
+ {
+ // The div should have snapped back to the previous position
+ if (targetScroller.scrollLeft != scrollPositionBeforeSnap)
+ testFailed("div did not snap back to proper location. Expected " + scrollPositionBeforeSnap + ", but got " + targetScroller.scrollLeft);
+ else
+ testPassed("div honored snap points.");
+
+ finishJSTest();
+ }
+
+ function scrollSnapTest()
+ {
+ scrollPositionBeforeSnap = targetScroller.scrollLeft;
+
+ var startPosX = targetScroller.offsetLeft + 20;
+ var startPosY = targetScroller.offsetTop + 20;
+ eventSender.monitorWheelEvents();
+ eventSender.mouseMoveTo(startPosX, startPosY); // Make sure we are just outside the iFrame
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'began', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'changed', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'changed', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'ended', 'none');
+ eventSender.callAfterScrollingCompletes(checkForScrollSnap);
+ }
+
+ function checkForScrollGlide()
+ {
+ // The div should have scrolled (glided) to the next snap point, which should be the width
+ // of the first div plus the root element's left margin (50 pixels).
+ if (targetScroller.scrollLeft == window.innerWidth + 50)
+ testPassed("div scrolled to next window.");
+ else
+ testFailed("div did not honor snap points. Expected " + window.innerWidth + ", but got " + targetScroller.scrollLeft);
+
+ setTimeout(scrollSnapTest, 0);
+ }
+
+ function scrollGlideTest()
+ {
+ scrollPositionBeforeGlide = targetScroller.scrollLeft;
+
+ var startPosX = targetScroller.offsetLeft + 20;
+ var startPosY = targetScroller.offsetTop + 20;
+ eventSender.monitorWheelEvents();
+ eventSender.mouseMoveTo(startPosX, startPosY); // Make sure we are just outside the iFrame
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'began', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'changed', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'changed', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'changed', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'ended', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'none', 'begin');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(-1, 0, 'none', 'continue');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'none', 'end');
+ eventSender.callAfterScrollingCompletes(checkForScrollGlide);
+ }
+
+ function onLoad()
+ {
+ targetScroller = document.scrollingElement;
+ if (window.eventSender) {
+ internals.setPlatformMomentumScrollingPredictionEnabled(false);
+ setTimeout(scrollGlideTest, 0);
+ } else {
+ var messageLocation = document.getElementById('item0');
+ var message = document.createElement('div');
+ message.innerHTML = "<p>To run this test manually, scroll the page horizontally. The page should "
+ + "alternate between uniform colors which fill the view.<p>";
+ messageLocation.appendChild(message);
+ }
+ }
+ </script>
+ </head>
+ <body _onload_="onLoad();" class="horizontalGallery">
+ <div id="item0" class="colorBox"><div id="console"></div></div>
+ <div id="item1" class="colorBox"></div>
+ <div id="item2" class="colorBox"></div>
+ <div id="item3" class="colorBox"></div>
+ <div id="item4" class="colorBox"></div>
+ <div id="item5" class="colorBox"></div>
+ </body>
+</html>
Added: trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-with-margin-expected.txt (0 => 269622)
--- trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-with-margin-expected.txt (rev 0)
+++ trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-with-margin-expected.txt 2020-11-10 09:28:40 UTC (rev 269622)
@@ -0,0 +1,6 @@
+PASS div scrolled to next window.
+PASS div honored snap points.
+PASS successfullyParsed is true
+
+TEST COMPLETE
+
Added: trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-with-margin.html (0 => 269622)
--- trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-with-margin.html (rev 0)
+++ trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-with-margin.html 2020-11-10 09:28:40 UTC (rev 269622)
@@ -0,0 +1,120 @@
+<!DOCTYPE HTML>
+<html>
+ <head>
+ <style>
+ html {
+ scroll-snap-type: y mandatory;
+ margin-top: 50px;
+ /* There's a different value for the bottom margin to ensure that
+ margins with different sizes are properly tested. */
+ margin-bottom: 100px;
+ }
+ .verticalGallery {
+ width: 100vw;
+ height: 600vh;
+ margin: 0;
+ padding: 0;
+ }
+ .colorBox {
+ height: 100vh;
+ width: 100vw;
+ float: left;
+ scroll-snap-align: start;
+ }
+ #item0 { background-color: red; }
+ #item1 { background-color: green; }
+ #item2 { background-color: blue; }
+ #item3 { background-color: aqua; }
+ #item4 { background-color: yellow; }
+ #item5 { background-color: fuchsia; }
+ </style>
+ <script src=""
+ <script>
+ window.jsTestIsAsync = true;
+
+ var scrollingTarget;
+ var scrollPositionBeforeGlide;
+ var scrollPositionBeforeSnap;
+
+ function checkForScrollSnap()
+ {
+ // The div should have snapped back to the previous position
+ if (scrollingTarget.scrollTop != scrollPositionBeforeSnap)
+ testFailed(`div did not snap back to proper location. (${scrollingTarget.scrollTop} vs. ${scrollPositionBeforeSnap})`);
+ else
+ testPassed("div honored snap points.");
+
+ finishJSTest();
+ }
+
+ function scrollSnapTest()
+ {
+ scrollPositionBeforeSnap = scrollingTarget.scrollTop;
+
+ var startPosX = scrollingTarget.offsetLeft + 20;
+ var startPosY = scrollingTarget.offsetTop + 20;
+ eventSender.monitorWheelEvents();
+ eventSender.mouseMoveTo(startPosX, startPosY); // Make sure we are just outside the iFrame
+ 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.callAfterScrollingCompletes(checkForScrollSnap);
+ }
+
+ function checkForScrollGlide()
+ {
+ // The div should have scrolled (glided) to the next snap point, which should be the height
+ // of the first div plus the root element's top margin (50 pixels).
+ if (scrollingTarget.scrollTop == window.innerHeight + 50)
+ testPassed("div scrolled to next window.");
+ else
+ testFailed(`div did not honor snap points. (${scrollingTarget.scrollTop} vs. ${window.innerHeight})`);
+
+ setTimeout(scrollSnapTest, 0);
+ }
+
+ function scrollGlideTest()
+ {
+ scrollPositionBeforeGlide = scrollingTarget.scrollTop;
+
+ var startPosX = scrollingTarget.offsetLeft + 20;
+ var startPosY = scrollingTarget.offsetTop + 20;
+ eventSender.monitorWheelEvents();
+ eventSender.mouseMoveTo(startPosX, startPosY); // Make sure we are just outside the iFrame
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'began', 'none');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, -1, 'changed', '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, -4, 'none', 'continue');
+ eventSender.mouseScrollByWithWheelAndMomentumPhases(0, 0, 'none', 'end');
+ eventSender.callAfterScrollingCompletes(checkForScrollGlide);
+ }
+
+ function onLoad()
+ {
+ scrollingTarget = document.scrollingElement;
+ if (window.eventSender) {
+ internals.setPlatformMomentumScrollingPredictionEnabled(false);
+ setTimeout(scrollGlideTest, 0);
+ } else {
+ var messageLocation = document.getElementById('item0');
+ var message = document.createElement('div');
+ message.innerHTML = "<p>To run this test manually, scroll the page vertically. The page should "
+ + "alternate between uniform colors which fill the view.<p>";
+ messageLocation.appendChild(message);
+ }
+ }
+ </script>
+ </head>
+ <body _onload_="onLoad();" class="verticalGallery">
+ <div id="item0" class="colorBox"><div id="console"></div></div>
+ <div id="item1" class="colorBox"></div>
+ <div id="item2" class="colorBox"></div>
+ <div id="item3" class="colorBox"></div>
+ <div id="item4" class="colorBox"></div>
+ <div id="item5" class="colorBox"></div>
+ </body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (269621 => 269622)
--- trunk/Source/WebCore/ChangeLog 2020-11-10 09:05:17 UTC (rev 269621)
+++ trunk/Source/WebCore/ChangeLog 2020-11-10 09:28:40 UTC (rev 269622)
@@ -1,3 +1,22 @@
+2020-11-10 Martin Robinson <mrobin...@igalia.com>
+
+ Scroll-snap on the root aligns to the body margin edge, not the viewport edge
+ https://bugs.webkit.org/show_bug.cgi?id=210476
+ <rdar://problem/61755103>
+
+ Reviewed by Simon Fraser.
+
+ When passing the frame viewport to updateSnapOffsetsForScrollableArea, be sure to put it
+ into the coordinate system of the root element padding box. This means offsetting it by the
+ margins of the root element.
+
+ Tests: tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-horizontal-with-margin.html
+ tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-with-margin.html
+
+ * page/FrameView.cpp:
+ (WebCore::FrameView::updateSnapOffsets): Offset viewport by top and left margins of the
+ root element.
+
2020-11-09 Tim Horton <timothy_hor...@apple.com>
Exceptions under PlatformCALayerCocoa::drawLayerContents with DisplayList-backed layers
Modified: trunk/Source/WebCore/page/FrameView.cpp (269621 => 269622)
--- trunk/Source/WebCore/page/FrameView.cpp 2020-11-10 09:05:17 UTC (rev 269621)
+++ trunk/Source/WebCore/page/FrameView.cpp 2020-11-10 09:28:40 UTC (rev 269622)
@@ -941,7 +941,13 @@
return;
}
+ // updateSnapOffsetsForScrollableArea calculates scroll offsets with all rectangles having their origin at the
+ // padding box rectangle of the scrollable element. Unlike for overflow:scroll, the FrameView viewport includes
+ // the root element margins. This means that we need to offset the viewport rectangle to make it relative to
+ // the padding box of the root element.
LayoutRect viewport = LayoutRect(IntPoint(), baseLayoutViewportSize());
+ viewport.move(-rootRenderer->marginLeft(), -rootRenderer->marginTop());
+
updateSnapOffsetsForScrollableArea(*this, *rootRenderer, *styleToUse, viewport);
}
