[webkit-changes] [279364] trunk

Tue, 29 Jun 2021 00:14:17 -0700

Title: [279364] trunk
Revision
279364
Author
[email protected]
Date
2021-06-29 00:14:08 -0700 (Tue, 29 Jun 2021)

Log Message

CSS scroll snap should allow scrolling to the middle of snap areas that overflow the snapport
https://bugs.webkit.org/show_bug.cgi?id=223021
<rdar://problem/75518606>

Patch by Martin Robinson <[email protected]> on 2021-06-29
Reviewed by Simon Fraser.

LayoutTests/imported/w3c:

* web-platform-tests/css/css-scroll-snap/overflowing-snap-areas-expected.txt: Update test
expectations to reflect newly passing tests.

Source/WebCore:

No new tests. This is covered by existing tests and, in particular, a WPT test:
    - web-platform-tests/css/css-scroll-snap/overflowing-snap-areas.html

* page/scrolling/ScrollSnapOffsetsInfo.cpp:
(WebCore::searchForPotentialSnapPoints): Record when the target snap offset happens to
fall in the interior of a snap area that overflows the snap port.
(WebCore::closestSnapOffsetWithInfoAndAxis): When this happens and we are in a spec-compliant
sitaution, just snap to the original target offset.

LayoutTests:

Update some existing tests that were relying on non-spec compliant behavior. These tests
used snap areas that were larger than the snapport and didn't expect that the parent
scroller could scroll into them.

* css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html: This test used snap areas that
overflow the snapport, thus meant that it relied on non-spec compliant behavior. Rework it so
that the snap areas do not overflow, maintaining the existing behavior.
* tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-then-horizontal.html: Ditto.
* tiled-drawing/scrolling/scroll-snap/scroll-snap-proximity-overflow.html: Ditto.

Modified Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (279363 => 279364)


--- trunk/LayoutTests/ChangeLog	2021-06-29 06:11:52 UTC (rev 279363)
+++ trunk/LayoutTests/ChangeLog	2021-06-29 07:14:08 UTC (rev 279364)
@@ -1,3 +1,21 @@
+2021-06-29  Martin Robinson  <[email protected]>
+
+        CSS scroll snap should allow scrolling to the middle of snap areas that overflow the snapport
+        https://bugs.webkit.org/show_bug.cgi?id=223021
+        <rdar://problem/75518606>
+
+        Reviewed by Simon Fraser.
+
+        Update some existing tests that were relying on non-spec compliant behavior. These tests
+        used snap areas that were larger than the snapport and didn't expect that the parent
+        scroller could scroll into them.
+
+        * css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html: This test used snap areas that
+        overflow the snapport, thus meant that it relied on non-spec compliant behavior. Rework it so
+        that the snap areas do not overflow, maintaining the existing behavior.
+        * tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-then-horizontal.html: Ditto.
+        * tiled-drawing/scrolling/scroll-snap/scroll-snap-proximity-overflow.html: Ditto.
+
 2021-06-28  Wenson Hsieh  <[email protected]>
 
         REGRESSION (r279310): Occasional crash when focusing login fields on iPad with a software keyboard

Modified: trunk/LayoutTests/css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html (279363 => 279364)


--- trunk/LayoutTests/css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html	2021-06-29 06:11:52 UTC (rev 279363)
+++ trunk/LayoutTests/css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html	2021-06-29 07:14:08 UTC (rev 279364)
@@ -34,21 +34,32 @@
                 scroll-snap-type: both mandatory;
             }
 
-            .block {
-                height: 1000px;
-                width: 250px;
-                scroll-snap-align: start;
-            }
-
             .horizontal-drawer {
                 height: 100%;
                 width: 2000px;
             }
 
-            .horizontal-drawer .block {
-                width: 1000px;
-                height: 250px;
+            .horizontal-drawer > div {
+                float: left;
+                width: 990px;
+                height: 100%;
             }
+
+            .horizontal-drawer > div.snap-point {
+                scroll-snap-align: start;
+                width: 10px;
+             }
+
+            #vertical-container > div {
+                width: 100%;
+                height: 990px;
+            }
+
+            #vertical-container > div.snap-point {
+                scroll-snap-align: start;
+                height: 10px;
+            }
+
         </style>
         <script src=""
         <script src=""
@@ -87,13 +98,17 @@
     <body _onload_="onLoad();">
         <div id="horizontal-container" class="container">
             <div class="horizontal-drawer">
-                <div class="block" style="float: left; background: #80475E"></div>
-                <div class="block" style="width: 450px; float: left; background: #CC5A71"></div>
+                <div class="snap-point" style="background: #80475E"></div>
+                <div></div>
+                <div class="snap-point" style="background: #CC5A71"></div>
+                <div></div>
             </div>
         </div>
         <div id="vertical-container" class="container">
-            <div class="block" style="background: #80475E"></div>
-            <div class="block" style="height: 450px; background: #CC5A71"></div>
+            <div class="snap-point" style="background: #80475E"></div>
+            <div></div>
+            <div class="snap-point" style="background: #CC5A71"></div>
+            <div></div>
         </div>
         <p id="console"></p>
         <script>

Modified: trunk/LayoutTests/imported/w3c/ChangeLog (279363 => 279364)


--- trunk/LayoutTests/imported/w3c/ChangeLog	2021-06-29 06:11:52 UTC (rev 279363)
+++ trunk/LayoutTests/imported/w3c/ChangeLog	2021-06-29 07:14:08 UTC (rev 279364)
@@ -1,3 +1,14 @@
+2021-06-29  Martin Robinson  <[email protected]>
+
+        CSS scroll snap should allow scrolling to the middle of snap areas that overflow the snapport
+        https://bugs.webkit.org/show_bug.cgi?id=223021
+        <rdar://problem/75518606>
+
+        Reviewed by Simon Fraser.
+
+        * web-platform-tests/css/css-scroll-snap/overflowing-snap-areas-expected.txt: Update test
+        expectations to reflect newly passing tests.
+
 2021-06-28  Darin Adler  <[email protected]>
 
         CSS parser "consume declaration" algorithm does not handle whitespace correctly

Modified: trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/overflowing-snap-areas-expected.txt (279363 => 279364)


--- trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/overflowing-snap-areas-expected.txt	2021-06-29 06:11:52 UTC (rev 279363)
+++ trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/overflowing-snap-areas-expected.txt	2021-06-29 07:14:08 UTC (rev 279364)
@@ -1,13 +1,13 @@
 
 PASS Snaps to the snap position if the snap area doesn't cover the snapport on x.
-FAIL Snaps to the snap position if the snap area covers the snapport on x on the right border. assert_equals: expected 2715 but got 200
-FAIL Snaps to the snap position if the snap area covers the snapport on x on the left border. assert_equals: expected 200 but got 2715
+PASS Snaps to the snap position if the snap area covers the snapport on x on the right border.
+PASS Snaps to the snap position if the snap area covers the snapport on x on the left border.
 PASS Snaps if the distance between the previous(400) and next(800) snap positions is smaller than snapport(500) on x.
 PASS Snaps if the distance between the previous(400) and next(800) snap positions is smaller than snapport(500) on y.
-FAIL Snap to current scroll position which is a valid snap position, as the snap area covers snapport on x and the distance between the previous(800) and next(1400) is larger than snapport(500). assert_equals: expected 900 but got 800
-FAIL Snap to current scroll position which is a valid snap position, as the snap area covers snapport on y and the distance between the previous(800) and next(1400) is larger than snapport(500). assert_equals: expected 900 but got 800
-FAIL Snap to current scroll position which is a valid snap position, as the snap area covers snapport on x and there is no subsequent snap positions. assert_equals: expected 1500 but got 1400
-FAIL Snap to current scroll position which is a valid snap position, as the snap area covers snapport on y and there is no subsequent snap positions. assert_equals: expected 1500 but got 1400
-FAIL Don't snap back even if scrollTo tries to scroll to positions which are outside of the scroll range and if a snap target element covers the snaport assert_equals: expected 3715 but got 2200
-FAIL Snap to current scroll position on x as the area is covering x axis.However, we snap to the specified snap position on y as the area is not covering y axis. assert_equals: expected 10 but got 0
+PASS Snap to current scroll position which is a valid snap position, as the snap area covers snapport on x and the distance between the previous(800) and next(1400) is larger than snapport(500).
+PASS Snap to current scroll position which is a valid snap position, as the snap area covers snapport on y and the distance between the previous(800) and next(1400) is larger than snapport(500).
+PASS Snap to current scroll position which is a valid snap position, as the snap area covers snapport on x and there is no subsequent snap positions.
+PASS Snap to current scroll position which is a valid snap position, as the snap area covers snapport on y and there is no subsequent snap positions.
+PASS Don't snap back even if scrollTo tries to scroll to positions which are outside of the scroll range and if a snap target element covers the snaport
+PASS Snap to current scroll position on x as the area is covering x axis.However, we snap to the specified snap position on y as the area is not covering y axis.

Modified: trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-then-horizontal.html (279363 => 279364)


--- trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-then-horizontal.html	2021-06-29 06:11:52 UTC (rev 279363)
+++ trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-then-horizontal.html	2021-06-29 07:14:08 UTC (rev 279364)
@@ -4,6 +4,7 @@
         <style>
             html {
                 scroll-snap-type: y mandatory;
+                margin: 0;
             }
             .verticalGallery {
                 width: 100vw;
@@ -12,7 +13,7 @@
                 padding: 0;
             }
             .colorBox {
-                height: 100vh;
+                height: 90vh;
                 width: 100vw;
                 float: left;
                 scroll-snap-align: start;

Modified: trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-proximity-overflow.html (279363 => 279364)


--- trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-proximity-overflow.html	2021-06-29 06:11:52 UTC (rev 279363)
+++ trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-proximity-overflow.html	2021-06-29 07:14:08 UTC (rev 279364)
@@ -13,21 +13,16 @@
                 position: absolute;
                 top: 0;
                 left: 0;
-                overflow-x: none;
                 overflow-y: scroll;
                 scroll-snap-type: y proximity;
-                -webkit-scroll-snap-type: proximity;
-                scroll-snap-type: proximity;
                 opacity: 0.5;
             }
 
             .area {
-                height: 600px;
-                width: 600px;
+                height: 100%;
+                width: 100%;
                 float: left;
                 scroll-snap-align: start;
-                -webkit-scroll-snap-coordinate: 0 0;
-                scroll-snap-coordinate: 0 0;
             }
         </style>
         <script>

Modified: trunk/Source/WebCore/ChangeLog (279363 => 279364)


--- trunk/Source/WebCore/ChangeLog	2021-06-29 06:11:52 UTC (rev 279363)
+++ trunk/Source/WebCore/ChangeLog	2021-06-29 07:14:08 UTC (rev 279364)
@@ -1,3 +1,20 @@
+2021-06-29  Martin Robinson  <[email protected]>
+
+        CSS scroll snap should allow scrolling to the middle of snap areas that overflow the snapport
+        https://bugs.webkit.org/show_bug.cgi?id=223021
+        <rdar://problem/75518606>
+
+        Reviewed by Simon Fraser.
+
+        No new tests. This is covered by existing tests and, in particular, a WPT test:
+            - web-platform-tests/css/css-scroll-snap/overflowing-snap-areas.html
+
+        * page/scrolling/ScrollSnapOffsetsInfo.cpp:
+        (WebCore::searchForPotentialSnapPoints): Record when the target snap offset happens to
+        fall in the interior of a snap area that overflows the snap port.
+        (WebCore::closestSnapOffsetWithInfoAndAxis): When this happens and we are in a spec-compliant
+        sitaution, just snap to the original target offset.
+
 2021-06-28  Xabier Rodriguez Calvar  <[email protected]>
 
         [GStreamer][EME] Fix resources release when a MediaKeySession is closed

Modified: trunk/Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp (279363 => 279364)


--- trunk/Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp	2021-06-29 06:11:52 UTC (rev 279363)
+++ trunk/Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp	2021-06-29 07:14:08 UTC (rev 279364)
@@ -45,13 +45,15 @@
     std::optional<std::pair<UnitType, unsigned>> previous;
     std::optional<std::pair<UnitType, unsigned>> next;
     std::optional<std::pair<UnitType, unsigned>> snapStop;
+    bool landedInsideSnapAreaThatConsumesViewport;
 };
 
 template <typename InfoType, typename UnitType>
-static PotentialSnapPointSearchResult<UnitType> searchForPotentialSnapPoints(const InfoType& info, ScrollEventAxis axis, UnitType destinationOffset, std::optional<UnitType> originalOffset)
+static PotentialSnapPointSearchResult<UnitType> searchForPotentialSnapPoints(const InfoType& info, ScrollEventAxis axis, UnitType viewportLength, UnitType destinationOffset, std::optional<UnitType> originalOffset)
 {
     const auto& snapOffsets = info.offsetsForAxis(axis);
     std::optional<std::pair<UnitType, unsigned>> previous, next, exact, snapStop;
+    bool landedInsideSnapAreaThatConsumesViewport = false;
 
     // A particular snap stop is better if it's between the original offset and destination offset and closer original
     // offset than the previously selected snap stop. We always want to stop at the snap stop closest to the original offset.
@@ -66,6 +68,12 @@
 
     for (unsigned i = 0; i < snapOffsets.size(); i++) {
         UnitType potentialSnapOffset = snapOffsets[i].offset;
+
+        const auto& snapArea = info.snapAreas[snapOffsets[i].snapAreaIndex];
+        auto snapAreaMin = axis == ScrollEventAxis::Horizontal ? snapArea.x() : snapArea.y();
+        auto snapAreaMax = axis == ScrollEventAxis::Horizontal ? snapArea.maxX() : snapArea.maxY();
+        landedInsideSnapAreaThatConsumesViewport |= snapAreaMin <= destinationOffset && snapAreaMax >= (destinationOffset + viewportLength);
+
         if (potentialSnapOffset == destinationOffset)
             exact = std::make_pair(potentialSnapOffset, i);
         else if (potentialSnapOffset < destinationOffset)
@@ -78,8 +86,8 @@
     }
 
     if (exact)
-        return { exact, exact, snapStop };
-    return { previous, next, snapStop };
+        return { exact, exact, snapStop, landedInsideSnapAreaThatConsumesViewport };
+    return { previous, next, snapStop, landedInsideSnapAreaThatConsumesViewport };
 }
 
 template <typename InfoType, typename SizeType, typename LayoutType>
@@ -90,11 +98,20 @@
     if (snapOffsets.isEmpty())
         return pairForNoSnapping;
 
-    auto [previous, next, snapStop] = searchForPotentialSnapPoints(info, axis, scrollDestinationOffset, originalOffsetForDirectionalSnapping);
+    auto viewportLength = axis == ScrollEventAxis::Horizontal ? viewportSize.width() : viewportSize.height();
+    auto [previous, next, snapStop, landedInsideSnapAreaThatConsumesViewport] = searchForPotentialSnapPoints(info, axis, viewportLength, scrollDestinationOffset, originalOffsetForDirectionalSnapping);
     if (snapStop)
         return *snapStop;
 
-    auto viewportLength = axis == ScrollEventAxis::Horizontal ? viewportSize.width() : viewportSize.height();
+    // From https://www.w3.org/TR/css-scroll-snap-1/#snap-overflow
+    // "If the snap area is larger than the snapport in a particular axis, then any scroll position
+    // in which the snap area covers the snapport, and the distance between the geometrically
+    // previous and subsequent snap positions in that axis is larger than size of the snapport in
+    // that axis, is a valid snap position in that axis. The UA may use the specified alignment as a
+    // more precise target for certain scroll operations (e.g. explicit paging)."
+    if (landedInsideSnapAreaThatConsumesViewport && (!previous || !next || ((*next).first - (*previous).first) >= viewportLength))
+        return pairForNoSnapping;
+
     auto isNearEnoughToOffsetForProximity = [&](LayoutType candidateSnapOffset) {
         if (info.strictness != ScrollSnapStrictness::Proximity)
             return true;

_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to