Changeset 268856 in webkit


Ignore:
Timestamp:
Oct 22, 2020 12:32:40 AM (21 months ago)
Author:
commit-queue@webkit.org
Message:

Scroll snap: don't create implicit snap points at scrollmin/scrollmax
​https://bugs.webkit.org/show_bug.cgi?id=216882
<rdar://problem/69443288>

Patch by Martin Robinson <​mrobinson@igalia.com> on 2020-10-22
Reviewed by Simon Fraser.

LayoutTests/imported/w3c:

Update expectations for these tests.

  • web-platform-tests/css/css-scroll-snap/nested-scrollIntoView-snaps-expected.txt:
  • web-platform-tests/css/css-scroll-snap/snap-after-relayout/adding-only-snap-area-expected.txt:
  • web-platform-tests/css/css-scroll-snap/snap-after-relayout/changing-scroll-snap-align-expected.txt:
  • web-platform-tests/css/css-scroll-snap/snap-after-relayout/snap-to-different-targets-expected.txt:
  • web-platform-tests/css/css-scroll-snap/snap-area-capturing-remove-scroll-container-expected.txt:

Source/WebCore:

  • page/scrolling/AxisScrollSnapOffsets.cpp:

(WebCore::updateSnapOffsetsForScrollableArea): No longer call adjustmentForTextAutosizing and
only conditionally adjust the scroll area by the scroll offset, becauase mapping to container
coordinates only applies container scroll offsets in the case that the container is not a
ScrollView.
(WebCore::adjustAxisSnapOffsetsForScrollExtent): Deleted.

LayoutTests:

  • TestExpectations: Mark some tests as passing.
  • css3/scroll-snap/nested-elements-expected.txt: Update results to eliminate implicit stops.
  • css3/scroll-snap/scroll-snap-children-with-padding-expected.txt: Ditto.
  • css3/scroll-snap/scroll-snap-children-with-scroll-snap-margin-expected.txt: Ditto.
  • css3/scroll-snap/scroll-snap-children-with-transforms-expected.txt: Ditto.
  • css3/scroll-snap/scroll-snap-elements-container-larger-than-children-expected.txt: Ditto.
  • css3/scroll-snap/scroll-snap-elements-container-larger-than-children-rtl-expected.txt: Ditto.
  • css3/scroll-snap/scroll-snap-offsets-expected.txt: Ditto.
  • css3/scroll-snap/scroll-snap-offsets-rtl-expected.txt: Ditto.
  • css3/scroll-snap/scroll-snap-positions-expected.txt: Ditto.
  • css3/scroll-snap/scroll-snap-with-scroll-padding-expected.txt: Ditto.
Location:
trunk
Files:
21 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r268851 r268856  
     12020-10-22  Martin Robinson  <mrobinson@igalia.com>
     2
     3        Scroll snap: don't create implicit snap points at scrollmin/scrollmax
     4        https://bugs.webkit.org/show_bug.cgi?id=216882
     5        <rdar://problem/69443288>
     6
     7        Reviewed by Simon Fraser.
     8
     9        * TestExpectations: Mark some tests as passing.
     10        * css3/scroll-snap/nested-elements-expected.txt: Update results to eliminate implicit stops.
     11        * css3/scroll-snap/scroll-snap-children-with-padding-expected.txt: Ditto.
     12        * css3/scroll-snap/scroll-snap-children-with-scroll-snap-margin-expected.txt: Ditto.
     13        * css3/scroll-snap/scroll-snap-children-with-transforms-expected.txt: Ditto.
     14        * css3/scroll-snap/scroll-snap-elements-container-larger-than-children-expected.txt: Ditto.
     15        * css3/scroll-snap/scroll-snap-elements-container-larger-than-children-rtl-expected.txt: Ditto.
     16        * css3/scroll-snap/scroll-snap-offsets-expected.txt: Ditto.
     17        * css3/scroll-snap/scroll-snap-offsets-rtl-expected.txt: Ditto.
     18        * css3/scroll-snap/scroll-snap-positions-expected.txt: Ditto.
     19        * css3/scroll-snap/scroll-snap-with-scroll-padding-expected.txt: Ditto.
     20
    1212020-10-21  Karl Rackler  <rackler@apple.com>
    222

  • trunk/LayoutTests/TestExpectations

    r268805 r268856  
    45104510imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/direction-rtl.html [ ImageOnlyFailure ]
    45114511imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/writing-mode-horizontal-tb.html [ ImageOnlyFailure ]
    4512 imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/writing-mode-vertical-lr.html [ ImageOnlyFailure ]
    4513 imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/writing-mode-vertical-rl.html [ ImageOnlyFailure ]
    45144512
    45154513# Cocoa-only

  • trunk/LayoutTests/css3/scroll-snap/nested-elements-expected.txt

    r217150 r268856  
    1 Scroll-snap offsets for 'container': vertical = { 0, 100, 215, 330, 445, 560, 675, 790, 905, 1020, 1135, 1250, 1365, 1480, 1595, 1710, 1825, 1940, 2055, 2170, 4900 }
     1Scroll-snap offsets for 'container': vertical = { 100, 215, 330, 445, 560, 675, 790, 905, 1020, 1135, 1250, 1365, 1480, 1595, 1710, 1825, 1940, 2055, 2170 }
    22PASS Scroll-snap offsets for 'invalidContainer': UNDEFINED
    33PASS successfullyParsed is true

  • trunk/LayoutTests/css3/scroll-snap/scroll-snap-children-with-padding-expected.txt

    r264891 r268856  
    1 Scroll-snap offsets are: vertical = { 0, 10, 625, 1240, 1660, 2275, 2890, 2900 }
     1Scroll-snap offsets are: vertical = { 10, 625, 1240, 1660, 2275, 2890 }
    22

  • trunk/LayoutTests/css3/scroll-snap/scroll-snap-children-with-scroll-snap-margin-expected.txt

    r210024 r268856  
    1 Scroll-snap offsets are: vertical = { 0, 700, 1410, 2120, 2830, 3540, 3600 }
    2 After resizing, the offsets should not have changed: vertical = { 0, 700, 1410, 2120, 2830, 3540, 3600 }
    3 After deflating scroll-snap-margin by 10px, the offsets are: vertical = { 0, 710, 1420, 2130, 2840, 3550, 3600 }
    4 After resizing, the offsets should not have changed: vertical = { 0, 710, 1420, 2130, 2840, 3550, 3600 }
     1Scroll-snap offsets are: vertical = { 0, 700, 1410, 2120, 2830, 3540 }
     2After resizing, the offsets should not have changed: vertical = { 0, 700, 1410, 2120, 2830, 3540 }
     3After deflating scroll-snap-margin by 10px, the offsets are: vertical = { 0, 710, 1420, 2130, 2840, 3550 }
     4After resizing, the offsets should not have changed: vertical = { 0, 710, 1420, 2130, 2840, 3550 }
    55

  • trunk/LayoutTests/css3/scroll-snap/scroll-snap-children-with-transforms-expected.txt

    r264891 r268856  
    1 Scroll-snap offsets are: vertical = { 0, 83, 752, 1457, 2233, 2880, 3673 }
    2 After resizing, the offsets should not have changed: vertical = { 0, 83, 752, 1457, 2233, 2880, 3673 }
    3 After changing alignment to `start`, the offsets are: vertical = { 0, 688, 1423, 2087, 2880, 3527, 3673 }
    4 After resizing, the offsets should not have changed: vertical = { 0, 688, 1423, 2087, 2880, 3527, 3673 }
     1Scroll-snap offsets are: vertical = { 83, 752, 1457, 2233, 2880, 3673 }
     2After resizing, the offsets should not have changed: vertical = { 83, 752, 1457, 2233, 2880, 3673 }
     3After changing alignment to `start`, the offsets are: vertical = { 0, 688, 1423, 2087, 2880, 3527 }
     4After resizing, the offsets should not have changed: vertical = { 0, 688, 1423, 2087, 2880, 3527 }
    55

  • trunk/LayoutTests/css3/scroll-snap/scroll-snap-elements-container-larger-than-children-expected.txt

    r217150 r268856  
    1 Scroll-snap offsets: horizontal = { 0, 200, 400, 600, 800, 1000, 1500 }
     1Scroll-snap offsets: horizontal = { 0, 200, 400, 600, 800, 1000 }
    22PASS successfullyParsed is true
    33

  • trunk/LayoutTests/css3/scroll-snap/scroll-snap-elements-container-larger-than-children-rtl-expected.txt

    r264908 r268856  
    1 Scroll-snap offsets: horizontal = { 0, 100, 300, 500, 700, 900, 1500 }
     1Scroll-snap offsets: horizontal = { 0, 100, 300, 500, 700, 900 }
    22PASS successfullyParsed is true
    33

  • trunk/LayoutTests/css3/scroll-snap/scroll-snap-offsets-expected.txt

    r264891 r268856  
    44Scroll-snap offsets for horizontalBorderedTarget: horizontal = { 0, 30, 60, 90, 120, 150 }
    55Scroll-snap offsets for verticalBorderedTarget: vertical = { 0, 30, 60, 90, 120, 150 }
    6 Scroll-snap offsets for horizontalPaddedTarget: horizontal = { 0, 20, 50, 80, 110, 140 }
    7 Scroll-snap offsets for verticalPaddedTarget: vertical = { 0, 15, 45, 75, 105, 135, 150 }
    8 Scroll-snap offsets for horizontalBorderedPaddedTarget: horizontal = { 0, 20, 50, 80, 110, 140 }
    9 Scroll-snap offsets for verticalBorderedPaddedTarget: vertical = { 0, 15, 45, 75, 105, 135, 150 }
    10 Scroll-snap offsets for horizontalRotatedTarget: horizontal = { 0, 20, 50, 80, 110, 140 }
    11 Scroll-snap offsets for verticalRotatedTarget: vertical = { 0, 15, 45, 75, 105, 135, 150 }
     6Scroll-snap offsets for horizontalPaddedTarget: horizontal = { 20, 50, 80, 110, 140 }
     7Scroll-snap offsets for verticalPaddedTarget: vertical = { 15, 45, 75, 105, 135, 150 }
     8Scroll-snap offsets for horizontalBorderedPaddedTarget: horizontal = { 20, 50, 80, 110, 140 }
     9Scroll-snap offsets for verticalBorderedPaddedTarget: vertical = { 15, 45, 75, 105, 135, 150 }
     10Scroll-snap offsets for horizontalRotatedTarget: horizontal = { 20, 50, 80, 110, 140 }
     11Scroll-snap offsets for verticalRotatedTarget: vertical = { 15, 45, 75, 105, 135, 150 }
    1212PASS successfullyParsed is true
    1313

  • trunk/LayoutTests/css3/scroll-snap/scroll-snap-offsets-rtl-expected.txt

    r264908 r268856  
    22Scroll-snap offsets for horizontalTarget: horizontal = { 0, 100, 200, 300, 400, 500 }
    33Scroll-snap offsets for horizontalBorderedTarget: horizontal = { 0, 100, 200, 300, 400, 500 }
    4 Scroll-snap offsets for horizontalPaddedTarget: horizontal = { 0, 70, 170, 270, 370, 470, 480 }
    5 Scroll-snap offsets for horizontalBorderedPaddedTarget: horizontal = { 0, 70, 170, 270, 370, 470, 480 }
    6 Scroll-snap offsets for horizontalRotatedTarget: horizontal = { 0, 70, 170, 270, 370, 470, 480 }
     4Scroll-snap offsets for horizontalPaddedTarget: horizontal = { 0, 70, 170, 270, 370, 470 }
     5Scroll-snap offsets for horizontalBorderedPaddedTarget: horizontal = { 0, 70, 170, 270, 370, 470 }
     6Scroll-snap offsets for horizontalRotatedTarget: horizontal = { 0, 70, 170, 270, 370, 470 }
    77PASS successfullyParsed is true
    88

  • trunk/LayoutTests/css3/scroll-snap/scroll-snap-positions-expected.txt

    r264891 r268856  
    1 Scroll-snap offsets for 'container': vertical = { 0, 73, 100, 330, 445, 560, 675, 763, 790, 993, 1020, 1250, 1310, 1365, 1425, 1480, 4425 }
     1Scroll-snap offsets for 'container': vertical = { 73, 100, 330, 445, 560, 675, 763, 790, 993, 1020, 1250, 1310, 1365, 1425, 1480 }
    22PASS successfullyParsed is true
    33

  • trunk/LayoutTests/css3/scroll-snap/scroll-snap-with-scroll-padding-expected.txt

    r210024 r268856  
    1 Scroll-snap offsets are: vertical = { 0, 670, 1390, 2110, 2830, 3550, 3600 }
    2 After resizing, the offsets should not have changed: vertical = { 0, 670, 1390, 2110, 2830, 3550, 3600 }
     1Scroll-snap offsets are: vertical = { 0, 670, 1390, 2110, 2830, 3550 }
     2After resizing, the offsets should not have changed: vertical = { 0, 670, 1390, 2110, 2830, 3550 }
    33After removing scroll-padding, the offsets are: vertical = { 0, 720, 1440, 2160, 2880, 3600 }
    44After resizing, the offsets should not have changed: vertical = { 0, 720, 1440, 2160, 2880, 3600 }

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r268853 r268856  
     12020-10-22  Martin Robinson  <mrobinson@igalia.com>
     2
     3        Scroll snap: don't create implicit snap points at scrollmin/scrollmax
     4        https://bugs.webkit.org/show_bug.cgi?id=216882
     5        <rdar://problem/69443288>
     6
     7        Reviewed by Simon Fraser.
     8
     9        Update expectations for these tests.
     10
     11        * web-platform-tests/css/css-scroll-snap/nested-scrollIntoView-snaps-expected.txt:
     12        * web-platform-tests/css/css-scroll-snap/snap-after-relayout/adding-only-snap-area-expected.txt:
     13        * web-platform-tests/css/css-scroll-snap/snap-after-relayout/changing-scroll-snap-align-expected.txt:
     14        * web-platform-tests/css/css-scroll-snap/snap-after-relayout/snap-to-different-targets-expected.txt:
     15        * web-platform-tests/css/css-scroll-snap/snap-area-capturing-remove-scroll-container-expected.txt:
     16
    1172020-10-21  Alex Christensen  <achristensen@webkit.org>
    218

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/nested-scrollIntoView-snaps-expected.txt

    r268250 r268856  
    11
    2 FAIL All the scrollers affected by scrollIntoView should land on a snap position if one exists. Otherwise, land according to the specified alignment assert_equals: ScrollIntoView lands on the target's snap position regardless of the specified alignment. expected 615 but got 800
     2FAIL All the scrollers affected by scrollIntoView should land on a snap position if one exists. Otherwise, land according to the specified alignment assert_equals: expected 0 but got 615
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-relayout/adding-only-snap-area-expected.txt

    r268250 r268856  
    11
    2 FAIL Adding a new snap area when there are none should make the scroller snap to it. assert_equals: expected 100 but got 0
     2PASS Adding a new snap area when there are none should make the scroller snap to it.
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-relayout/changing-scroll-snap-align-expected.txt

    r268250 r268856  
    22FAIL Changing the current target's snap alignment should make the scroller resnap to it even if another snap position is closer to the current offset assert_equals: expected 100 but got 0
    33FAIL Removing the current target's snap alignment should make the scroller resnap to a new snap area. assert_equals: expected 100 but got 0
    4 FAIL Changing an element snap alignment from none to start should make thescroller resnap. assert_equals: expected 100 but got 0
    5 FAIL Changing an element snap alignment from none to start by adding a class should make the scroller resnap. assert_equals: expected 100 but got 0
     4PASS Changing an element snap alignment from none to start should make thescroller resnap.
     5PASS Changing an element snap alignment from none to start by adding a class should make the scroller resnap.
    66

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-relayout/snap-to-different-targets-expected.txt

    r268250 r268856  
    11
    2 FAIL Scroller should snap to at least one of the targets if unable to snap toboth after a layout change. assert_equals: expected 200 but got 0
     2FAIL Scroller should snap to at least one of the targets if unable to snap toboth after a layout change. assert_equals: expected 200 but got 400
    33

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-area-capturing-remove-scroll-container-expected.txt

    r268250 r268856  
    11
    2 FAIL Making a snap container not scrollable should promote the next scrollable ancestor to become a snap container. assert_equals: expected 300 but got 10
     2FAIL Making a snap container not scrollable should promote the next scrollable ancestor to become a snap container. assert_equals: expected 300 but got 310
    33

  • trunk/LayoutTests/platform/ios/css3/scroll-snap/nested-elements-expected.txt

    r224214 r268856  
    1 Scroll-snap offsets for 'container': vertical = { 0, 100, 215, 330, 445, 560, 675, 790, 905, 1020, 1135, 1250, 1365, 1480, 1595, 1710, 1825, 1940, 2055, 2170, 4885 }
     1Scroll-snap offsets for 'container': vertical = { 100, 215, 330, 445, 560, 675, 790, 905, 1020, 1135, 1250, 1365, 1480, 1595, 1710, 1825, 1940, 2055, 2170 }
    22PASS Scroll-snap offsets for 'invalidContainer': UNDEFINED
    33PASS successfullyParsed is true

  • trunk/Source/WebCore/ChangeLog

    r268853 r268856  
     12020-10-22  Martin Robinson  <mrobinson@igalia.com>
     2
     3        Scroll snap: don't create implicit snap points at scrollmin/scrollmax
     4        https://bugs.webkit.org/show_bug.cgi?id=216882
     5        <rdar://problem/69443288>
     6
     7        Reviewed by Simon Fraser.
     8
     9        * page/scrolling/AxisScrollSnapOffsets.cpp:
     10        (WebCore::updateSnapOffsetsForScrollableArea): No longer call adjustmentForTextAutosizing and
     11        only conditionally adjust the scroll area by the scroll offset, becauase mapping to container
     12        coordinates only applies container scroll offsets in the case that the container is not a
     13        ScrollView.
     14        (WebCore::adjustAxisSnapOffsetsForScrollExtent): Deleted.
     15
    1162020-10-21  Alex Christensen  <achristensen@webkit.org>
    217

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

    r264908 r268856  
    137137}
    138138
    139 static void adjustAxisSnapOffsetsForScrollExtent(Vector<LayoutUnit>& snapOffsets, float maxScrollExtent)
    140 {
    141     if (snapOffsets.isEmpty())
    142         return;
    143 
    144     std::sort(snapOffsets.begin(), snapOffsets.end());
    145     if (snapOffsets.last() != maxScrollExtent)
    146         snapOffsets.append(maxScrollExtent);
    147     if (snapOffsets.first())
    148         snapOffsets.insert(0, 0);
    149 }
    150 
    151139static void computeAxisProximitySnapOffsetRanges(const Vector<LayoutUnit>& snapOffsets, Vector<ScrollOffsetRange<LayoutUnit>>& offsetRanges, LayoutUnit scrollPortAxisLength)
    152140{
    … …  
    207195        // FIXME: For now, just consider whether the scroller is RTL. The behavior of LTR boxes inside a RTL scroller is poorly defined: https://github.com/w3c/csswg-drafts/issues/5361.
    208196        auto scrollSnapArea = LayoutRect(child->localToContainerQuad(FloatQuad(child->borderBoundingBox()), scrollingElement.renderBox()).boundingBox());
    209         scrollSnapArea.moveBy(scrollPosition);
     197
     198        // localToContainerQuad will transform the scroll snap area by the scroll position, except in the case that this position is
     199        // coming from a ScrollView. We want the transformed area, but without scroll position taken into account.
     200        if (!scrollableArea.isScrollView())
     201            scrollSnapArea.moveBy(scrollPosition);
     202
    210203        scrollSnapArea = computeScrollSnapPortOrAreaRect(scrollSnapArea, child->style().scrollSnapMargin(), InsetOrOutset::Outset);
    211204        LOG_WITH_STREAM(ScrollSnap, stream << "    Considering scroll snap target area " << scrollSnapArea);
    … …  
    230223
    231224    if (!horizontalSnapOffsets.isEmpty()) {
    232         adjustAxisSnapOffsetsForScrollExtent(horizontalSnapOffsets, maxScrollOffset.x());
     225        std::sort(horizontalSnapOffsets.begin(), horizontalSnapOffsets.end());
     226        if (scrollSnapType.strictness == ScrollSnapStrictness::Proximity)
     227            computeAxisProximitySnapOffsetRanges(horizontalSnapOffsets, horizontalSnapOffsetRanges, scrollSnapPort.width());
     228
    233229        LOG_WITH_STREAM(ScrollSnap, stream << " => Computed horizontal scroll snap offsets: " << horizontalSnapOffsets);
    234230        LOG_WITH_STREAM(ScrollSnap, stream << " => Computed horizontal scroll snap offset ranges: " << horizontalSnapOffsetRanges);
    235         if (scrollSnapType.strictness == ScrollSnapStrictness::Proximity)
    236             computeAxisProximitySnapOffsetRanges(horizontalSnapOffsets, horizontalSnapOffsetRanges, scrollSnapPort.width());
    237231
    238232        scrollableArea.setHorizontalSnapOffsets(horizontalSnapOffsets);
    … …  
    242236
    243237    if (!verticalSnapOffsets.isEmpty()) {
    244         adjustAxisSnapOffsetsForScrollExtent(verticalSnapOffsets, maxScrollOffset.y());
     238        std::sort(verticalSnapOffsets.begin(), verticalSnapOffsets.end());
     239        if (scrollSnapType.strictness == ScrollSnapStrictness::Proximity)
     240            computeAxisProximitySnapOffsetRanges(verticalSnapOffsets, verticalSnapOffsetRanges, scrollSnapPort.height());
     241
    245242        LOG_WITH_STREAM(ScrollSnap, stream << " => Computed vertical scroll snap offsets: " << verticalSnapOffsets);
    246243        LOG_WITH_STREAM(ScrollSnap, stream << " => Computed vertical scroll snap offset ranges: " << verticalSnapOffsetRanges);
    247         if (scrollSnapType.strictness == ScrollSnapStrictness::Proximity)
    248             computeAxisProximitySnapOffsetRanges(verticalSnapOffsets, verticalSnapOffsetRanges, scrollSnapPort.height());
    249244
    250245        scrollableArea.setVerticalSnapOffsets(verticalSnapOffsets);
Note: See TracChangeset for help on using the changeset viewer.