Changeset 279714 in webkit

Timestamp:

Jul 8, 2021 12:23:32 AM (13 months ago)

Author:

commit-queue@webkit.org

Message:

[css-scroll-snap] scroll-snap-align should use the box's writing-mode when the box is larger than the snapport
​https://bugs.webkit.org/show_bug.cgi?id=227743

Patch by Martin Robinson <​mrobinson@igalia.com> on 2021-07-08
Reviewed by Simon Fraser.

Source/WebCore:

No new tests. This fixes an existing WPT test:

• imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/scroll-snap-writing-mode-000.html

• page/scrolling/ScrollSnapOffsetsInfo.cpp:

(WebCore::axesFlippedForWritingModeAndDirection): Added this helper which abstracts the process of
determining if the x-axis and y-axis are flipped given the style's writing mode and text direction.
(WebCore::updateSnapOffsetsForScrollableArea): When the inline axis of the box defining the snap area
is larger than the snapport, we use the box's writing mode and text-direction to determine if the
x-axis and y-axis are flipped.

LayoutTests:

• TestExpectations: Unskip a test which is now passing.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/TestExpectations (modified) (1 diff)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp (modified) (5 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2021-07-08  Martin Robinson  <mrobinson@igalia.com>
+
+        [css-scroll-snap] scroll-snap-align should use the box's writing-mode when the box is larger than the snapport
+        https://bugs.webkit.org/show_bug.cgi?id=227743
+
+        Reviewed by Simon Fraser.
+
+        * TestExpectations: Unskip a test which is now passing.
+
 2021-07-07  Chris Dumez  <cdumez@apple.com>
 

trunk/LayoutTests/TestExpectations

@@ -4726 +4726 @@
 imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-snap-001.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-snap-002.html [ ImageOnlyFailure ]
-imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/scroll-snap-writing-mode-000.html [ ImageOnlyFailure ]
 webkit.org/b/218325 imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-margin-001.html [ Pass ImageOnlyFailure ]
 webkit.org/b/218325 imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-margin-003.html [ Pass ImageOnlyFailure ]

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-07-08  Martin Robinson  <mrobinson@igalia.com>
+
+        [css-scroll-snap] scroll-snap-align should use the box's writing-mode when the box is larger than the snapport
+        https://bugs.webkit.org/show_bug.cgi?id=227743
+
+        Reviewed by Simon Fraser.
+
+        No new tests. This fixes an existing WPT test:
+           - imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/scroll-snap-writing-mode-000.html
+
+        * page/scrolling/ScrollSnapOffsetsInfo.cpp:
+        (WebCore::axesFlippedForWritingModeAndDirection): Added this helper which abstracts the process of
+        determining if the x-axis and y-axis are flipped given the style's writing mode and text direction.
+        (WebCore::updateSnapOffsetsForScrollableArea): When the inline axis of the box defining the snap area
+        is larger than the snapport, we use the box's writing mode and text-direction to determine if the
+        x-axis and y-axis are flipped.
+
 2021-07-08  Fujii Hironori  <Hironori.Fujii@sony.com>
 

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

@@ -200 +200 @@
 }
 
+static std::pair<bool, bool> axesFlippedForWritingModeAndDirection(WritingMode writingMode, TextDirection textDirection)
+{
+    // text-direction flips the inline axis and writing-mode can flip the block axis. Whether or
+    // not the writing-mode is vertical determines the physical orientation of the block and inline axes.
+    bool hasVerticalWritingMode = isVerticalWritingMode(writingMode);
+    bool blockAxisFlipped = isFlippedWritingMode(writingMode);
+    bool inlineAxisFlipped = textDirection == TextDirection::RTL;
+    return std::make_pair(hasVerticalWritingMode ? blockAxisFlipped : inlineAxisFlipped, hasVerticalWritingMode ? inlineAxisFlipped : blockAxisFlipped);
+}
+
 void updateSnapOffsetsForScrollableArea(ScrollableArea& scrollableArea, const RenderBox& scrollingElementBox, const RenderStyle& scrollingElementStyle, LayoutRect viewportRectInBorderBoxCoordinates, WritingMode writingMode, TextDirection textDirection)
 {
@@ -224 +234 @@
     auto scrollPosition = LayoutPoint { scrollableArea.scrollPosition() };
 
-    // text-direction flips the inline axis and writing-mode can flip the block axis. Whether or
-    // not the writing-mode is vertical determines the physical orientation of the block and inline axes.
+    auto [scrollerXAxisFlipped, scrollerYAxisFlipped] = axesFlippedForWritingModeAndDirection(writingMode, textDirection);
     bool scrollerHasVerticalWritingMode = isVerticalWritingMode(writingMode);
-    bool blockAxisFlipped = isFlippedWritingMode(writingMode);
-    bool inlineAxisFlipped = textDirection == TextDirection::RTL;
-    bool xAxisFlipped = scrollerHasVerticalWritingMode ? blockAxisFlipped : inlineAxisFlipped;
-    bool yAxisFlipped = scrollerHasVerticalWritingMode ? inlineAxisFlipped : blockAxisFlipped;
-
     bool hasHorizontalSnapOffsets = scrollSnapType.axis == ScrollSnapAxis::Both || scrollSnapType.axis == ScrollSnapAxis::XAxis;
     bool hasVerticalSnapOffsets = scrollSnapType.axis == ScrollSnapAxis::Both || scrollSnapType.axis == ScrollSnapAxis::YAxis;
@@ -264 +268 @@
         auto stop = child->style().scrollSnapStop();
 
+        // From https://drafts.csswg.org/css-scroll-snap-1/#scroll-snap-align:
+        // "Start and end alignments are resolved with respect to the writing mode of the snap container unless the
+        // scroll snap area is larger than the snapport, in which case they are resolved with respect to the writing
+        // mode of the box itself."
+        bool areaXAxisFlipped = scrollerXAxisFlipped;
+        bool areaYAxisFlipped = scrollerYAxisFlipped;
+        bool areaHasVerticalWritingMode = isVerticalWritingMode(child->style().writingMode());
+        if ((areaHasVerticalWritingMode && scrollSnapArea.height() > scrollSnapPort.height()) || (!areaHasVerticalWritingMode && scrollSnapArea.width() > scrollSnapPort.width()))
+            std::tie(areaXAxisFlipped, areaYAxisFlipped) = axesFlippedForWritingModeAndDirection(child->style().writingMode(), child->style().direction());
+
         ScrollSnapAxisAlignType xAlign = scrollerHasVerticalWritingMode ? alignment.blockAlign : alignment.inlineAlign;
         ScrollSnapAxisAlignType yAlign = scrollerHasVerticalWritingMode ? alignment.inlineAlign : alignment.blockAlign;
@@ -271 +285 @@
         if (!snapsHorizontally && !snapsVertically)
             continue;
-
         // The scroll snap area is defined via its scroll position, so convert the snap area rectangle to be relative to scroll offsets.
         auto snapAreaOriginRelativeToBorderEdge = scrollSnapArea.location() - scrollSnapPort.location();
@@ -278 +291 @@
 
         if (snapsHorizontally) {
-            auto absoluteScrollXPosition = computeScrollSnapAlignOffset(scrollSnapArea.x(), scrollSnapArea.maxX(), xAlign, xAxisFlipped) - computeScrollSnapAlignOffset(scrollSnapPort.x(), scrollSnapPort.maxX(), xAlign, xAxisFlipped);
+            auto absoluteScrollXPosition = computeScrollSnapAlignOffset(scrollSnapArea.x(), scrollSnapArea.maxX(), xAlign, areaXAxisFlipped) - computeScrollSnapAlignOffset(scrollSnapPort.x(), scrollSnapPort.maxX(), xAlign, areaXAxisFlipped);
             auto absoluteScrollOffset = clampTo<int>(scrollableArea.scrollOffsetFromPosition({ roundToInt(absoluteScrollXPosition), 0 }).x(), 0, maxScrollOffset.x());
             addOrUpdateStopForSnapOffset(horizontalSnapOffsetsMap, { absoluteScrollOffset, stop, snapAreas.size() - 1 });
         }
         if (snapsVertically) {
-            auto absoluteScrollYPosition = computeScrollSnapAlignOffset(scrollSnapArea.y(), scrollSnapArea.maxY(), yAlign, yAxisFlipped) - computeScrollSnapAlignOffset(scrollSnapPort.y(), scrollSnapPort.maxY(), yAlign, yAxisFlipped);
+            auto absoluteScrollYPosition = computeScrollSnapAlignOffset(scrollSnapArea.y(), scrollSnapArea.maxY(), yAlign, areaYAxisFlipped) - computeScrollSnapAlignOffset(scrollSnapPort.y(), scrollSnapPort.maxY(), yAlign, areaYAxisFlipped);
             auto absoluteScrollOffset = clampTo<int>(scrollableArea.scrollOffsetFromPosition({ 0, roundToInt(absoluteScrollYPosition) }).y(), 0, maxScrollOffset.y());
             addOrUpdateStopForSnapOffset(verticalSnapOffsetsMap, { absoluteScrollOffset, stop, snapAreas.size() - 1 });