Choose closest covering position for large snap areas
This patch modifies the logic to find snap positions so that when a scroller is scrolled to a position which only partially covers a snap
area larger than itself, if it snaps to this large snap area, it snaps
to the nearest boundary of that snap area where the snap area covers the
snap port, rather than doing a jump to honor the specified scroll-snap-align.
Below are a few notes about changes made to tests:
snap-area-overflow-boundary.html is corrected so that both test cases
correctly verify the scroll offset when snapped to the lower element.
The scroll amount in the second test case is also adjusted to ensure
it is enough to snap to the lower div.
The expectations at the end of
scroll-on-large-element-not-covering-snapport.tentative.html are
updated to account for choosing a snap position different from the
scroll-snap-align-specified position for snap areas larger than their
snapports.
scroll-snap-type-on-root-element.html is updated to account for
scrollbar width since snapping to the right edge of the target is
valid.
This patch picks new programmatic scroll offsets for
snap-to-visible-areas-margin-both.html so that the top-left of
intersection of the right-top and left-bottom targets' scroll-margins
is closer than the bottom-right of the intersection.
scrollend-with-snap-on-fractional-offset.html and
scroll-start-with-scroll-snap-tentative.html are adjusted to use snap
area sizes that don't cover the snap port.
Bug: 1420762
Change-Id: I72d18b8ca44ea5aef015e59b67e4de34a66d8a1e
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4806971
Reviewed-by: Robert Flack <flackr@chromium.org>
Commit-Queue: David Awogbemila <awogbemila@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1193634}
diff --git a/css/css-scroll-snap-2/scroll-start/scroll-start-with-scroll-snap.tentative.html b/css/css-scroll-snap-2/scroll-start/scroll-start-with-scroll-snap.tentative.html
index b813fcc..8bdac30 100644
--- a/css/css-scroll-snap-2/scroll-start/scroll-start-with-scroll-snap.tentative.html
+++ b/css/css-scroll-snap-2/scroll-start/scroll-start-with-scroll-snap.tentative.html
@@ -17,21 +17,27 @@
}
.spacer {
- height: 100px;
+ height: 200px;
width: 100px;
}
.scroller {
- height: 100px;
+ height: 220px;
width: 100px;
overflow: scroll;
- scroll-start: 200px;
scroll-snap-type: both mandatory;
}
+ #single_snap_scroller {
+ scroll-start: 100%;
+ }
+ #multi_snap_scroller {
+ scroll-start: 350px;
+ }
+
.snap_point {
width: 100px;
- height: 100px;
+ height: 200px;
scroll-snap-align: start;
}
</style>
@@ -53,11 +59,11 @@
}, "snap overrides scroll-start position");
test((t) => {
- // scroll-start sets the initial scroll offset to the top of the third
- // snap_point, so the scroller snaps to the third snap_point.
+ // scroll-start sets the initial scroll offset to 350px which is closer to
+ // the third snap point than the second, so the scroller should snap to
+ // the third snap_point.
assert_equals(multi_snap_scroller.scrollTop,
- snap_point_1.getBoundingClientRect().height +
- snap_point_2.getBoundingClientRect().height,
+ multi_snap_scroller.scrollHeight - multi_snap_scroller.clientHeight,
"scroller snaps to snap point closer to start position.");
}, "scroller snaps based on scroll-start position");
</script>
diff --git a/css/css-scroll-snap/input/snap-area-overflow-boundary.html b/css/css-scroll-snap/input/snap-area-overflow-boundary.html
index c497845..1f00353 100644
--- a/css/css-scroll-snap/input/snap-area-overflow-boundary.html
+++ b/css/css-scroll-snap/input/snap-area-overflow-boundary.html
@@ -3,6 +3,9 @@
<title></title>
<meta name="assert" content="Test passes if snap is to the nearest edge">
<style>
+ body {
+ margin: 0px;
+ }
#scroller {
scroll-snap-type: block mandatory;
overflow-y: scroll;
@@ -105,7 +108,7 @@
await keyPress(scroller, "ArrowDown");
await waitForAnimationEnd(scrollTop);
assert_equals(scroller.scrollTop,
- next.clientTop,
+ next.offsetTop,
'Advance to next snap-area');
}, "Keyboard scrolling with vertical snap-area overflow");
@@ -126,7 +129,7 @@
// Target position for wheel scroll overshoots the boundary of the snap-area.
// Ensure that we stop at the boundary.
- const scrollAmount =
+ let scrollAmount =
target.clientHeight - scroller.clientHeight - scroller.scrollTop + 1;
await new test_driver.Actions()
@@ -137,13 +140,15 @@
'End boundary of snap-area is valid snap target');
// Must not get stuck at a snap position. Since already at the end of the
- // snap area, we should advance to the next.
+ // snap area, we should advance to the next. scrollAmount must be enough to
+ // advance to next snap position.
+ scrollAmount = next.clientHeight / 2 + 10 /* margin-bottom */;
await new test_driver.Actions()
- .scroll(50, 50, 0, 50, {origin: scroller})
+ .scroll(50, 50, 0, scrollAmount, {origin: scroller})
.send();
await waitForAnimationEnd(scrollTop);
assert_equals(scroller.scrollTop,
- next.clientTop,
+ next.offsetTop,
'Advance to next snap-area');
}, "Mouse-wheel scrolling with vertical snap-area overflow");
diff --git a/css/css-scroll-snap/scroll-on-large-element-not-covering-snapport.tentative.html b/css/css-scroll-snap/scroll-on-large-element-not-covering-snapport.tentative.html
index 2db00d4..df4fc20 100644
--- a/css/css-scroll-snap/scroll-on-large-element-not-covering-snapport.tentative.html
+++ b/css/css-scroll-snap/scroll-on-large-element-not-covering-snapport.tentative.html
@@ -80,12 +80,9 @@
// Scroll back a bit.
scroller.scrollBy(0, -10);
- // Now, snap back to the third snap point because at the moment when scrolling
- // up by 10px, the large third snap target element isn't covering over the
- // snapport, i.e. only bottom 10px of the large element is in the snapport.
- // See https://github.com/w3c/csswg-drafts/issues/7262
+ // This should snap to the bottom of the 3rd snap area and not all the way
+ // back up to its top.
assert_equals(scroller.scrollLeft, 0);
- assert_equals(scroller.scrollTop, 80);
-}, 'There\'s no valid snap positions on large element if it doesn\'t cover ' +
- 'the snapport');
+ assert_equals(scroller.scrollTop, 880);
+}, "snaps to bottom edge of large snap area that doesn't cover the snap port.");
</script>
diff --git a/css/css-scroll-snap/scroll-snap-type-on-root-element.html b/css/css-scroll-snap/scroll-snap-type-on-root-element.html
index 2556ed9..a45ac92 100644
--- a/css/css-scroll-snap/scroll-snap-type-on-root-element.html
+++ b/css/css-scroll-snap/scroll-snap-type-on-root-element.html
@@ -4,12 +4,13 @@
<meta name="viewport" content="width=device-width,initial-scale=1,minimum-scale=1">
<script src="/resources/testharness.js"></script>
<script src="/resources/testharnessreport.js"></script>
+<body>
<style>
-html {
+body {
height: 8000px;
width: 3000px;
+ margin: 0px;
}
-
#target {
position: absolute;
background-color: blue;
@@ -68,7 +69,13 @@
window.scrollTo(200, 1800);
- assert_equals(document.scrollingElement.scrollLeft, 100, "inline should snap");
+ // Though the target's width is 100vw, there may be room to scroll the window
+ // within the target because of the scrollbar width and the browser may snap
+ // to the right edge (instead of the left edge of the target, since it is
+ // closer to the offset of 200 and still a valid snap point) within this room.
+ const scrollbar_width = window.innerWidth - document.documentElement.clientWidth;
+ assert_approx_equals(document.scrollingElement.scrollLeft, 100, scrollbar_width, "inline should snap");
assert_equals(document.scrollingElement.scrollTop, 1800, "block should not snap");
}, "The writing-mode (horizontal-tb) on the body is used ");
</script>
+</body>
diff --git a/css/css-scroll-snap/snap-after-initial-layout/scroll-snap-writing-mode-000-ref.html b/css/css-scroll-snap/snap-after-initial-layout/scroll-snap-writing-mode-000-ref.html
index 26b622e..428eb3f 100644
--- a/css/css-scroll-snap/snap-after-initial-layout/scroll-snap-writing-mode-000-ref.html
+++ b/css/css-scroll-snap/snap-after-initial-layout/scroll-snap-writing-mode-000-ref.html
@@ -35,17 +35,6 @@
.ltr { direction: ltr; }
.rtl { direction: rtl; }
-.TB.large.invert .target { top: auto; }
-.LR.large.invert .target { left: auto; }
-.RL.large.invert .target { right: auto; }
-
-.TB.ltr.large.invert .target { left: auto; }
-.TB.rtl.large.invert .target { right: auto; }
-.LR.ltr.large.invert .target { top: auto; }
-.LR.rtl.large.invert .target { bottom: auto; }
-.RL.ltr.large.invert .target { top: auto; }
-.RL.rtl.large.invert .target { bottom: auto; }
-
/* not absolutizing the border colors, so that the test passes even if css-logical is not supported; */
.large.invert {
border: solid silver;
@@ -53,10 +42,40 @@
border-inline-end-color: blue;
}
+.TB.large.invert .target::before { top: auto; }
+.LR.large.invert .target::before { left: auto; }
+.RL.large.invert .target::before { right: auto; }
+
+.TB.ltr.large.invert .target::before { left: auto; }
+.TB.rtl.large.invert .target::before { right: auto; }
+.LR.ltr.large.invert .target::before { top: auto; }
+.LR.rtl.large.invert .target::before { bottom: auto; }
+.RL.ltr.large.invert .target::before { top: auto; }
+.RL.rtl.large.invert .target::before { bottom: auto; }
+
+.large.invert .target::before {
+ width: 9px;
+ height: 9px;
+ background: orange;
+ top: 0; left: 0; right: 0; bottom: 0;
+ position: absolute;
+ content: '';
+}
+
+.large.invert .target {
+ display: block;
+ background: none;
+ width: 30px;
+ height: 30px;
+ border-block-start: 20px solid red;
+ border-inline-start: 20px solid red;
+}
+
</style>
<p>Test passes if there is an orange square tucked into each blue corner without gaps,
-and there is no red.
+and there is no red, except for the large inverted cases which should have red
+in the silver corner and smaller orange boxes in the blue corner.
<div class="wrapper">
<!-- Simple Small Cases -->
@@ -87,6 +106,7 @@
<!-- Target-inverted Small Cases
This row should be identical to the previous. -->
+
<div class="scroller TB ltr small invert">
<div class="target"></div>
</div>
@@ -165,3 +185,4 @@
</div>
</div> <!-- wrapper -->
+
diff --git a/css/css-scroll-snap/snap-after-initial-layout/scroll-snap-writing-mode-000.html b/css/css-scroll-snap/snap-after-initial-layout/scroll-snap-writing-mode-000.html
index e5d3dd9..f44317c 100644
--- a/css/css-scroll-snap/snap-after-initial-layout/scroll-snap-writing-mode-000.html
+++ b/css/css-scroll-snap/snap-after-initial-layout/scroll-snap-writing-mode-000.html
@@ -78,7 +78,8 @@
</style>
<p>Test passes if there is an orange square tucked into each blue corner without gaps,
-and there is no red.
+ and there is no red, except for the large inverted cases which should have red
+ in the silver corner and smaller orange boxes in the blue corner.
<div class="wrapper">
<!-- Simple Small Cases -->
diff --git a/css/css-scroll-snap/snap-into-covering-area.html b/css/css-scroll-snap/snap-into-covering-area.html
new file mode 100644
index 0000000..2cc8741
--- /dev/null
+++ b/css/css-scroll-snap/snap-into-covering-area.html
@@ -0,0 +1,77 @@
+<!DOCTYPE html>
+<html>
+
+<head>
+ <link rel="help" href="https://drafts.csswg.org/css-scroll-snap-1/#scroll-snap-type" />
+ <script src="/resources/testharness.js"></script>
+ <script src="/resources/testharnessreport.js"></script>
+ <script src="/resources/testdriver.js"></script>
+ <script src="/resources/testdriver-actions.js"></script>
+ <script src="/resources/testdriver-vendor.js"></script>
+ <script src="/dom/events/scrolling/scroll_support.js"></script>
+</head>
+
+<body>
+ <style>
+ #scroller {
+ overflow: scroll;
+ height: 500px;
+ width: 500px;
+ background-color: blue;
+ scroll-snap-type: y mandatory;
+ position: absolute;
+ }
+
+ .snap_point {
+ scroll-snap-align: start;
+ width: 40%;
+ position: relative;
+ left: 30%;
+ }
+
+ .big {
+ height: 1000%;
+ background-color: pink;
+ border: solid 1px red;
+ }
+
+ .small {
+ height: 50%;
+ background-color: purple;
+ border: solid 1px black;
+ }
+ </style>
+ <div id="scroller">
+ <div class="big snap_point" id="big_snap_point"></div>
+ <div class="small snap_point">
+ <button id="scrollerButton">scrollerButton</button>
+ </div>
+ </div>
+ <script>
+ promise_test(async(t) => {
+ const x = scroller.clientWidth / 2;
+ const y = scroller.clientHeight / 2;
+
+ // Scroll all the way down to the smaller snap area which doesn't cover
+ // the snapport.
+ let scrollend_promise = new Promise((resolve) => {
+ scroller.addEventListener("scrollend", resolve);
+ });
+ scroller.scrollTop = scroller.scrollHeight;
+ await scrollend_promise;
+
+ // Scroll up with one press of the arrow-up button.
+ scrollend_promise = new Promise((resolve) => {
+ scroller.addEventListener("scrollend", resolve);
+ });
+ const arrowUp = '\uE013';
+ await test_driver.send_keys(scrollerButton, arrowUp);
+
+ await scrollend_promise;
+ assert_equals(scroller.scrollTop, big_snap_point.offsetHeight - scroller.clientHeight,
+ "scroller is snapped to the bottom of the larger snap area, not the top");
+ });
+ </script>
+</body>
+
+</html>
\ No newline at end of file
diff --git a/css/css-scroll-snap/snap-to-visible-areas-margin-both.html b/css/css-scroll-snap/snap-to-visible-areas-margin-both.html
index f148036..4c3c3c1 100644
--- a/css/css-scroll-snap/snap-to-visible-areas-margin-both.html
+++ b/css/css-scroll-snap/snap-to-visible-areas-margin-both.html
@@ -65,10 +65,14 @@
scroller.scrollTo(0, 0);
assert_equals(scroller.scrollLeft, 0);
assert_equals(scroller.scrollTop, 0);
- scroller.scrollTo(500, 600);
+ // 750 and 650 are picked as those are closer to top left of the intersection
+ // (800, 800) of the snap areas where the browser should snap. This makes the
+ // intersection a closer snap option than a covering option that the browser
+ // might choose where the snapport is aligned on the bottom and right.
+ scroller.scrollTo(650, 750);
assert_equals(scroller.scrollLeft, 800);
assert_equals(scroller.scrollTop, 800);
- scroller.scrollTo(600, 500);
+ scroller.scrollTo(750, 650);
assert_equals(scroller.scrollLeft, 800);
assert_equals(scroller.scrollTop, 800);
}, 'Snap to area such that only the scroll margin from both axes\' areas are \
diff --git a/dom/events/scrolling/scrollend-with-snap-on-fractional-offset.html b/dom/events/scrolling/scrollend-with-snap-on-fractional-offset.html
index 74dcb0f..d1f5030 100644
--- a/dom/events/scrolling/scrollend-with-snap-on-fractional-offset.html
+++ b/dom/events/scrolling/scrollend-with-snap-on-fractional-offset.html
@@ -16,7 +16,7 @@
.box {
scroll-snap-align: start;
- width: 600px;
+ width: 400px;
position: absolute;
top: 200px;
}
