Changeset 279364 in webkit


Ignore:
Timestamp:
Jun 29, 2021 12:14:08 AM (13 months ago)
Author:
commit-queue@webkit.org
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 <​mrobinson@igalia.com> 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.
Location:
trunk
Files:
8 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r279361 r279364  
     12021-06-29  Martin Robinson  <mrobinson@igalia.com>
     2
     3        CSS scroll snap should allow scrolling to the middle of snap areas that overflow the snapport
     4        https://bugs.webkit.org/show_bug.cgi?id=223021
     5        <rdar://problem/75518606>
     6
     7        Reviewed by Simon Fraser.
     8
     9        Update some existing tests that were relying on non-spec compliant behavior. These tests
     10        used snap areas that were larger than the snapport and didn't expect that the parent
     11        scroller could scroll into them.
     12
     13        * css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html: This test used snap areas that
     14        overflow the snapport, thus meant that it relied on non-spec compliant behavior. Rework it so
     15        that the snap areas do not overflow, maintaining the existing behavior.
     16        * tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-then-horizontal.html: Ditto.
     17        * tiled-drawing/scrolling/scroll-snap/scroll-snap-proximity-overflow.html: Ditto.
     18
    1192021-06-28  Wenson Hsieh  <wenson_hsieh@apple.com>
    220

  • trunk/LayoutTests/css3/scroll-snap/scroll-snap-click-scrollbar-gutter.html

    r273690 r279364  
    3535            }
    3636
    37             .block {
    38                 height: 1000px;
    39                 width: 250px;
    40                 scroll-snap-align: start;
    41             }
    42 
    4337            .horizontal-drawer {
    4438                height: 100%;
    … …  
    4640            }
    4741
    48             .horizontal-drawer .block {
    49                 width: 1000px;
    50                 height: 250px;
     42            .horizontal-drawer > div {
     43                float: left;
     44                width: 990px;
     45                height: 100%;
    5146            }
     47
     48            .horizontal-drawer > div.snap-point {
     49                scroll-snap-align: start;
     50                width: 10px;
     51             }
     52
     53            #vertical-container > div {
     54                width: 100%;
     55                height: 990px;
     56            }
     57
     58            #vertical-container > div.snap-point {
     59                scroll-snap-align: start;
     60                height: 10px;
     61            }
     62
    5263        </style>
    5364        <script src="../../resources/js-test.js"></script>
    … …  
    8899        <div id="horizontal-container" class="container">
    89100            <div class="horizontal-drawer">
    90                 <div class="block" style="float: left; background: #80475E"></div>
    91                 <div class="block" style="width: 450px; float: left; background: #CC5A71"></div>
     101                <div class="snap-point" style="background: #80475E"></div>
     102                <div></div>
     103                <div class="snap-point" style="background: #CC5A71"></div>
     104                <div></div>
    92105            </div>
    93106        </div>
    94107        <div id="vertical-container" class="container">
    95             <div class="block" style="background: #80475E"></div>
    96             <div class="block" style="height: 450px; background: #CC5A71"></div>
     108            <div class="snap-point" style="background: #80475E"></div>
     109            <div></div>
     110            <div class="snap-point" style="background: #CC5A71"></div>
     111            <div></div>
    97112        </div>
    98113        <p id="console"></p>

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r279358 r279364  
     12021-06-29  Martin Robinson  <mrobinson@igalia.com>
     2
     3        CSS scroll snap should allow scrolling to the middle of snap areas that overflow the snapport
     4        https://bugs.webkit.org/show_bug.cgi?id=223021
     5        <rdar://problem/75518606>
     6
     7        Reviewed by Simon Fraser.
     8
     9        * web-platform-tests/css/css-scroll-snap/overflowing-snap-areas-expected.txt: Update test
     10        expectations to reflect newly passing tests.
     11
    1122021-06-28  Darin Adler  <darin@apple.com>
    213

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/overflowing-snap-areas-expected.txt

    r271439 r279364  
    11
    22PASS Snaps to the snap position if the snap area doesn't cover the snapport on x.
    3 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
    4 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
     3PASS Snaps to the snap position if the snap area covers the snapport on x on the right border.
     4PASS Snaps to the snap position if the snap area covers the snapport on x on the left border.
    55PASS Snaps if the distance between the previous(400) and next(800) snap positions is smaller than snapport(500) on x.
    66PASS Snaps if the distance between the previous(400) and next(800) snap positions is smaller than snapport(500) on y.
    7 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
    8 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
    9 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
    10 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
    11 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
    12 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
     7PASS 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).
     8PASS 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).
     9PASS 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.
     10PASS 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.
     11PASS 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
     12PASS 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.
    1313

  • trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-mandatory-mainframe-vertical-then-horizontal.html

    r269730 r279364  
    55            html {
    66                scroll-snap-type: y mandatory;
     7                margin: 0;
    78            }
    89            .verticalGallery {
    … …  
    1314            }
    1415            .colorBox {
    15                 height: 100vh;
     16                height: 90vh;
    1617                width: 100vw;
    1718                float: left;

  • trunk/LayoutTests/tiled-drawing/scrolling/scroll-snap/scroll-snap-proximity-overflow.html

    r210560 r279364  
    1414                top: 0;
    1515                left: 0;
    16                 overflow-x: none;
    1716                overflow-y: scroll;
    1817                scroll-snap-type: y proximity;
    19                 -webkit-scroll-snap-type: proximity;
    20                 scroll-snap-type: proximity;
    2118                opacity: 0.5;
    2219            }
    2320
    2421            .area {
    25                 height: 600px;
    26                 width: 600px;
     22                height: 100%;
     23                width: 100%;
    2724                float: left;
    2825                scroll-snap-align: start;
    29                 -webkit-scroll-snap-coordinate: 0 0;
    30                 scroll-snap-coordinate: 0 0;
    3126            }
    3227        </style>

  • trunk/Source/WebCore/ChangeLog

    r279363 r279364  
     12021-06-29  Martin Robinson  <mrobinson@igalia.com>
     2
     3        CSS scroll snap should allow scrolling to the middle of snap areas that overflow the snapport
     4        https://bugs.webkit.org/show_bug.cgi?id=223021
     5        <rdar://problem/75518606>
     6
     7        Reviewed by Simon Fraser.
     8
     9        No new tests. This is covered by existing tests and, in particular, a WPT test:
     10            - web-platform-tests/css/css-scroll-snap/overflowing-snap-areas.html
     11
     12        * page/scrolling/ScrollSnapOffsetsInfo.cpp:
     13        (WebCore::searchForPotentialSnapPoints): Record when the target snap offset happens to
     14        fall in the interior of a snap area that overflows the snap port.
     15        (WebCore::closestSnapOffsetWithInfoAndAxis): When this happens and we are in a spec-compliant
     16        sitaution, just snap to the original target offset.
     17
    1182021-06-28  Xabier Rodriguez Calvar  <calvaris@igalia.com>
    219

  • trunk/Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp

    r279272 r279364  
    4646    std::optional<std::pair<UnitType, unsigned>> next;
    4747    std::optional<std::pair<UnitType, unsigned>> snapStop;
     48    bool landedInsideSnapAreaThatConsumesViewport;
    4849};
    4950
    5051template <typename InfoType, typename UnitType>
    51 static PotentialSnapPointSearchResult<UnitType> searchForPotentialSnapPoints(const InfoType& info, ScrollEventAxis axis, UnitType destinationOffset, std::optional<UnitType> originalOffset)
     52static PotentialSnapPointSearchResult<UnitType> searchForPotentialSnapPoints(const InfoType& info, ScrollEventAxis axis, UnitType viewportLength, UnitType destinationOffset, std::optional<UnitType> originalOffset)
    5253{
    5354    const auto& snapOffsets = info.offsetsForAxis(axis);
    5455    std::optional<std::pair<UnitType, unsigned>> previous, next, exact, snapStop;
     56    bool landedInsideSnapAreaThatConsumesViewport = false;
    5557
    5658    // A particular snap stop is better if it's between the original offset and destination offset and closer original
    … …  
    6769    for (unsigned i = 0; i < snapOffsets.size(); i++) {
    6870        UnitType potentialSnapOffset = snapOffsets[i].offset;
     71
     72        const auto& snapArea = info.snapAreas[snapOffsets[i].snapAreaIndex];
     73        auto snapAreaMin = axis == ScrollEventAxis::Horizontal ? snapArea.x() : snapArea.y();
     74        auto snapAreaMax = axis == ScrollEventAxis::Horizontal ? snapArea.maxX() : snapArea.maxY();
     75        landedInsideSnapAreaThatConsumesViewport |= snapAreaMin <= destinationOffset && snapAreaMax >= (destinationOffset + viewportLength);
     76
    6977        if (potentialSnapOffset == destinationOffset)
    7078            exact = std::make_pair(potentialSnapOffset, i);
    … …  
    7987
    8088    if (exact)
    81         return { exact, exact, snapStop };
    82     return { previous, next, snapStop };
     89        return { exact, exact, snapStop, landedInsideSnapAreaThatConsumesViewport };
     90    return { previous, next, snapStop, landedInsideSnapAreaThatConsumesViewport };
    8391}
    8492
    … …  
    9199        return pairForNoSnapping;
    92100
    93     auto [previous, next, snapStop] = searchForPotentialSnapPoints(info, axis, scrollDestinationOffset, originalOffsetForDirectionalSnapping);
     101    auto viewportLength = axis == ScrollEventAxis::Horizontal ? viewportSize.width() : viewportSize.height();
     102    auto [previous, next, snapStop, landedInsideSnapAreaThatConsumesViewport] = searchForPotentialSnapPoints(info, axis, viewportLength, scrollDestinationOffset, originalOffsetForDirectionalSnapping);
    94103    if (snapStop)
    95104        return *snapStop;
    96105
    97     auto viewportLength = axis == ScrollEventAxis::Horizontal ? viewportSize.width() : viewportSize.height();
     106    // From https://www.w3.org/TR/css-scroll-snap-1/#snap-overflow
     107    // "If the snap area is larger than the snapport in a particular axis, then any scroll position
     108    // in which the snap area covers the snapport, and the distance between the geometrically
     109    // previous and subsequent snap positions in that axis is larger than size of the snapport in
     110    // that axis, is a valid snap position in that axis. The UA may use the specified alignment as a
     111    // more precise target for certain scroll operations (e.g. explicit paging)."
     112    if (landedInsideSnapAreaThatConsumesViewport && (!previous || !next || ((*next).first - (*previous).first) >= viewportLength))
     113        return pairForNoSnapping;
     114
    98115    auto isNearEnoughToOffsetForProximity = [&](LayoutType candidateSnapOffset) {
    99116        if (info.strictness != ScrollSnapStrictness::Proximity)
Note: See TracChangeset for help on using the changeset viewer.