Attachment #429156 for bug #226010

View Details Raw Unified Return to Bug 226010

| Differences between



2021-05-20  Martin Robinson  <mrobinson@igalia.com>

        [css-scroll-snap] Scroll snap is broken with non-horizontal writing modes
        https://bugs.webkit.org/show_bug.cgi?id=226010

        Reviewed by NOBODY (OOPS!).

        Fix issues related to vertical writing modes and scroll snap.

        This change fixes three existing WPT tests.
            imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element.html
            imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/writing-mode-vertical-lr.html
            imported/w3c/web-platform-tests/css/css-scroll-snap/snap-inline-block.html

        * css/CSSComputedStyleDeclaration.cpp:
        (WebCore::valueForScrollSnapAlignment): Update to reflect new member names of ScrollSnapAlign.
        * page/FrameView.cpp:
        (WebCore::FrameView::updateSnapOffsets): Pass in the text direction and writing mode of the
        container, allowing the values specified on the body to override those specified on the root
        element.
        * page/scrolling/ScrollSnapOffsetsInfo.cpp:
        (WebCore::updateSnapOffsetsForScrollableArea): Properly handle the writing mode and the
        text direction of the container.
        * page/scrolling/ScrollSnapOffsetsInfo.h: Update function signature.
        * rendering/RenderLayerModelObject.cpp:
        (WebCore::scrollSnapContainerRequiresUpdateForStyleUpdate): Pass in the writing mode and
        text direction of the scrolling container.
        * rendering/RenderLayerScrollableArea.cpp:
        (WebCore::RenderLayerScrollableArea::updateSnapOffsets): Update to reflect new member names
        of ScrollSnapAlign.
        * rendering/style/RenderStyle.cpp:
        (WebCore::RenderStyle::hasSnapPosition const): Ditto.
        * rendering/style/StyleScrollSnapPoints.h: Change the name of the members of ScrollSnapAlign
        to match what is described in the specification. The values provided are for block and
        inline directions, but depending on the scroll container.
        (WebCore::operator==): Ditto.
        * style/StyleBuilderConverter.h:
        (WebCore::Style::BuilderConverter::convertScrollSnapAlign): Ditto.

2021-05-18  Cameron McCormack  <heycam@apple.com>

        Record gradient and pattern filled canvas text in the correct coordinate system.



static Ref<CSSValueList> valueForScrollSnapAlignment(const ScrollSnapAlign& alignment)
{
    auto value = CSSValueList::createSpaceSeparated();
    value->append(CSSPrimitiveValue::create(alignment.blockAlign));
    if (alignment.inlineAlign != alignment.blockAlign)
        value->append(CSSPrimitiveValue::create(alignment.inlineAlign));
    return value;
}


Lines 948-954 void FrameView::updateSnapOffsets() a/Source/WebCore/page/FrameView.cpp_sec1
- a/Source/WebCore/page/FrameView.cpp -1 / +9 lines
948	LayoutRect viewport = LayoutRect(IntPoint(), baseLayoutViewportSize());	948	LayoutRect viewport = LayoutRect(IntPoint(), baseLayoutViewportSize());
949	viewport.move(-rootRenderer->marginLeft(), -rootRenderer->marginTop());	949	viewport.move(-rootRenderer->marginLeft(), -rootRenderer->marginTop());
950		950
951	updateSnapOffsetsForScrollableArea(this, rootRenderer, *styleToUse, viewport);	951	// From https://www.w3.org/TR/css-writing-modes-3/
		952	// "As a special case for handling HTML documents, if the root element has a body child element
		953	// [HTML], the used value of the of writing-mode and direction properties on root element are
		954	// taken from the computed writing-mode and direction of the first such child element instead of
		955	// from the root element’s own values."
		956	WritingMode writingMode = (bodyRenderer ? bodyRenderer->style() : rootRenderer->style()).writingMode();
		957	TextDirection direction = (bodyRenderer ? bodyRenderer->style() : rootRenderer->style()).direction();
		958
		959	updateSnapOffsetsForScrollableArea(this, rootRenderer, *styleToUse, viewport, writingMode, direction);
952	}	960	}
953		961
954	bool FrameView::isScrollSnapInProgress() const	962	bool FrameView::isScrollSnapInProgress() const



    }
}

void updateSnapOffsetsForScrollableArea(ScrollableArea& scrollableArea, const RenderBox& scrollingElementBox, const RenderStyle& scrollingElementStyle, LayoutRect viewportRectInBorderBoxCoordinates, WritingMode writingMode, TextDirection textDirection)
{
    auto scrollSnapType = scrollingElementStyle.scrollSnapType();
    const auto& boxesWithScrollSnapPositions = scrollingElementBox.view().boxesWithScrollSnapPositions();

    HashMap<float, SnapOffset<LayoutUnit>> verticalSnapOffsetsMap;
    HashMap<float, SnapOffset<LayoutUnit>> horizontalSnapOffsetsMap;
    Vector<LayoutRect> snapAreas;
    bool hasHorizontalSnapOffsets = scrollSnapType.axis == ScrollSnapAxis::Both || scrollSnapType.axis == ScrollSnapAxis::XAxis || scrollSnapType.axis == ScrollSnapAxis::Inline;
    bool hasVerticalSnapOffsets = scrollSnapType.axis == ScrollSnapAxis::Both || scrollSnapType.axis == ScrollSnapAxis::YAxis || scrollSnapType.axis == ScrollSnapAxis::Block;

    auto maxScrollOffset = scrollableArea.maximumScrollOffset();
    auto scrollPosition = LayoutPoint { scrollableArea.scrollPosition() };
    bool scrollerIsRTL = textDirection == TextDirection::RTL || writingMode == WritingMode::RightToLeft;
    bool scrollerHasVerticalWritingMode = isVerticalWritingMode(writingMode);

    bool hasHorizontalSnapOffsets = scrollSnapType.axis == ScrollSnapAxis::Both || scrollSnapType.axis == ScrollSnapAxis::XAxis;
    bool hasVerticalSnapOffsets = scrollSnapType.axis == ScrollSnapAxis::Both || scrollSnapType.axis == ScrollSnapAxis::YAxis;
    if (scrollSnapType.axis == ScrollSnapAxis::Block) {
        hasHorizontalSnapOffsets = scrollerHasVerticalWritingMode;
        hasVerticalSnapOffsets = !scrollerHasVerticalWritingMode;
    }
    if (scrollSnapType.axis == ScrollSnapAxis::Inline) {
        hasHorizontalSnapOffsets = !scrollerHasVerticalWritingMode;
        hasVerticalSnapOffsets = scrollerHasVerticalWritingMode;
    }

    // The bounds of the scrolling container's snap port, where the top left of the scrolling container's border box is the origin.
    auto scrollSnapPort = computeScrollSnapPortOrAreaRect(viewportRectInBorderBoxCoordinates, scrollingElementStyle.scrollPadding(), InsetOrOutset::Inset);

        auto alignment = child->style().scrollSnapAlign();
        auto stop = child->style().scrollSnapStop();

        ScrollSnapAxisAlignType xAlign = !scrollerHasVerticalWritingMode ? alignment.inlineAlign : alignment.blockAlign;
        ScrollSnapAxisAlignType yAlign = !scrollerHasVerticalWritingMode ? alignment.blockAlign : alignment.inlineAlign;
        bool snapsHorizontally = hasHorizontalSnapOffsets && xAlign != ScrollSnapAxisAlignType::None;
        bool snapsVertically = hasVerticalSnapOffsets && yAlign != ScrollSnapAxisAlignType::None;

        if (!snapsHorizontally && !snapsVertically)
            continue;


        snapAreas.append(scrollSnapAreaAsOffsets);

        if (snapsHorizontally) {
            auto absoluteScrollXPosition = computeScrollSnapAlignOffset(scrollSnapArea.x(), scrollSnapArea.maxX(), xAlign, scrollerIsRTL) - computeScrollSnapAlignOffset(scrollSnapPort.x(), scrollSnapPort.maxX(), xAlign, scrollerIsRTL);
            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, false) - computeScrollSnapAlignOffset(scrollSnapPort.y(), scrollSnapPort.maxY(), yAlign, false);
            auto absoluteScrollOffset = clampTo<int>(scrollableArea.scrollOffsetFromPosition({ 0, roundToInt(absoluteScrollYPosition) }).y(), 0, maxScrollOffset.y());
            addOrUpdateStopForSnapOffset(verticalSnapOffsetsMap, { absoluteScrollOffset, stop, snapAreas.size() - 1 });
        }

        if (!snapAreas.isEmpty())
            LOG_WITH_STREAM(ScrollSnap, stream << " => Computed snap areas: " << snapAreas);
    }

    auto compareSnapOffsets = [](const SnapOffset<LayoutUnit>& a, const SnapOffset<LayoutUnit>& b)

        return a.offset < b.offset;
    };

    if (!snapAreas.isEmpty())
        LOG_WITH_STREAM(ScrollSnap, stream << " => Computed snap areas: " << snapAreas);

    Vector<SnapOffset<LayoutUnit>> horizontalSnapOffsets = copyToVector(horizontalSnapOffsetsMap.values());
    if (!horizontalSnapOffsets.isEmpty()) {
        std::sort(horizontalSnapOffsets.begin(), horizontalSnapOffsets.end(), compareSnapOffsets);

Lines 96-102 const unsigned invalidSnapOffsetIndex = UINT_MAX; a/Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.h_sec1
- a/Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.h -1 / +1 lines
96	// Update the snap offsets for this scrollable area, given the RenderBox of the scroll container, the RenderStyle	96	// Update the snap offsets for this scrollable area, given the RenderBox of the scroll container, the RenderStyle
97	// which defines the scroll-snap properties, and the viewport rectangle with the origin at the top left of	97	// which defines the scroll-snap properties, and the viewport rectangle with the origin at the top left of
98	// the scrolling container's border box.	98	// the scrolling container's border box.
99	void updateSnapOffsetsForScrollableArea(ScrollableArea&, const RenderBox& scrollingElementBox, const RenderStyle& scrollingElementStyle, LayoutRect viewportRectInBorderBoxCoordinates);	99	void updateSnapOffsetsForScrollableArea(ScrollableArea&, const RenderBox& scrollingElementBox, const RenderStyle& scrollingElementStyle, LayoutRect viewportRectInBorderBoxCoordinates, WritingMode, TextDirection);
100		100
101	template <typename T> WTF::TextStream& operator<<(WTF::TextStream& ts, SnapOffset<T> offset)	101	template <typename T> WTF::TextStream& operator<<(WTF::TextStream& ts, SnapOffset<T> offset)
102	{	102	{

Lines 110-116 void RenderLayerModelObject::styleWillChange(StyleDifference diff, const RenderS a/Source/WebCore/rendering/RenderLayerModelObject.cpp_sec1
- a/Source/WebCore/rendering/RenderLayerModelObject.cpp -1 / +1 lines
110	#if ENABLE(CSS_SCROLL_SNAP)	110	#if ENABLE(CSS_SCROLL_SNAP)
111	static bool scrollSnapContainerRequiresUpdateForStyleUpdate(const RenderStyle& oldStyle, const RenderStyle& newStyle)	111	static bool scrollSnapContainerRequiresUpdateForStyleUpdate(const RenderStyle& oldStyle, const RenderStyle& newStyle)
112	{	112	{
113	return oldStyle.scrollPadding() != newStyle.scrollPadding() \|\| oldStyle.scrollSnapType() != newStyle.scrollSnapType();	113	return oldStyle.scrollPadding() != newStyle.scrollPadding() \|\| oldStyle.scrollSnapType() != newStyle.scrollSnapType() \|\| oldStyle.writingMode() != newStyle.writingMode() \|\| oldStyle.direction() != newStyle.direction();
114	}	114	}
115	#endif	115	#endif
116		116



        return;

    RenderBox* box = m_layer.enclosingElement()->renderBox();
    fprintf(stderr, "vertical: %i\n", box->style().isVerticalWritingMode());
    updateSnapOffsetsForScrollableArea(*this, *box, box->style(), box->paddingBoxRect(), box->style().writingMode(), box->style().direction());
}

bool RenderLayerScrollableArea::isScrollSnapInProgress() const

Lines 2573-2579 void RenderStyle::setScrollSnapStop(const ScrollSnapStop stop) a/Source/WebCore/rendering/style/RenderStyle.cpp_sec1
- a/Source/WebCore/rendering/style/RenderStyle.cpp -1 / +1 lines
2573	bool RenderStyle::hasSnapPosition() const	2573	bool RenderStyle::hasSnapPosition() const
2574	{	2574	{
2575	const ScrollSnapAlign& alignment = this->scrollSnapAlign();	2575	const ScrollSnapAlign& alignment = this->scrollSnapAlign();
2576	return alignment.x != ScrollSnapAxisAlignType::None \|\| alignment.y != ScrollSnapAxisAlignType::None;	2576	return alignment.blockAlign != ScrollSnapAxisAlignType::None \|\| alignment.inlineAlign != ScrollSnapAxisAlignType::None;
2577	}	2577	}
2578	#endif	2578	#endif
2579		2579



inline bool operator!=(const ScrollSnapType& a, const ScrollSnapType& b) { return !(a == b); }

struct ScrollSnapAlign {
    ScrollSnapAxisAlignType blockAlign { ScrollSnapAxisAlignType::None };
    ScrollSnapAxisAlignType inlineAlign { ScrollSnapAxisAlignType::None };
};

inline bool operator==(const ScrollSnapAlign& a, const ScrollSnapAlign& b)
{
    return a.blockAlign == b.blockAlign && a.inlineAlign == b.inlineAlign;
}

inline bool operator!=(const ScrollSnapAlign& a, const ScrollSnapAlign& b) { return !(a == b); }



{
    auto& values = downcast<CSSValueList>(value);
    ScrollSnapAlign alignment;
    alignment.blockAlign = downcast<CSSPrimitiveValue>(*values.item(0));
    if (values.length() == 1)
        alignment.inlineAlign = alignment.blockAlign;
    else
        alignment.inlineAlign = downcast<CSSPrimitiveValue>(*values.item(1));
    return alignment;
}




2021-05-20  Martin Robinson  <mrobinson@igalia.com>

        [css-scroll-snap] Scroll snap is broken with non-horizontal writing modes
        https://bugs.webkit.org/show_bug.cgi?id=226010

        Reviewed by NOBODY (OOPS!).

        * TestExpectations: Mark one test as passing.

2021-05-18  Cameron McCormack  <heycam@apple.com>

        Record gradient and pattern filled canvas text in the correct coordinate system.



2021-05-20  Martin Robinson  <mrobinson@igalia.com>

        [css-scroll-snap] Scroll snap is broken with non-horizontal writing modes
        https://bugs.webkit.org/show_bug.cgi?id=226010

        Reviewed by NOBODY (OOPS!).

        * web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element-expected.txt: Update expectations to mark tests as passing.
        * web-platform-tests/css/css-scroll-snap/snap-inline-block-expected.txt: Ditto.

2021-05-18  Keith Miller  <keith_miller@apple.com>

        Temporarily revert r276592 as it breaks some native apps

Lines 4540-4546 imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-snap-003.html a/LayoutTests/TestExpectations_sec1
- a/LayoutTests/TestExpectations -1 lines
4540	imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/direction-rtl.html [ ImageOnlyFailure ]	4540	imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/direction-rtl.html [ ImageOnlyFailure ]
4541	imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/scroll-snap-initial-layout-000.html [ ImageOnlyFailure ]	4541	imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/scroll-snap-initial-layout-000.html [ ImageOnlyFailure ]
4542	imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/scroll-snap-writing-mode-000.html [ ImageOnlyFailure ]	4542	imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/scroll-snap-writing-mode-000.html [ ImageOnlyFailure ]
4543	imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/writing-mode-vertical-lr.html [ ImageOnlyFailure ]
4544	webkit.org/b/218325 imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-margin-001.html [ Pass ImageOnlyFailure ]	4543	webkit.org/b/218325 imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-margin-001.html [ Pass ImageOnlyFailure ]
4545	webkit.org/b/218325 imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-margin-003.html [ Pass ImageOnlyFailure ]	4544	webkit.org/b/218325 imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-target-margin-003.html [ Pass ImageOnlyFailure ]
4546		4545

Lines 1-5 a/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element-expected.txt_sec1
- a/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element-expected.txt -1 / +1 lines
1		1
2	PASS The scroll-snap-type on the root element is applied	2	PASS The scroll-snap-type on the root element is applied
3	FAIL The writing-mode (vertical-lr) on the body is used assert_equals: inline should snap expected 515 but got 800	3	PASS The writing-mode (vertical-lr) on the body is used
4	PASS The writing-mode (horizontal-tb) on the body is used	4	PASS The writing-mode (horizontal-tb) on the body is used
5		5




PASS Snaps correctly for horizontal-tb writing mode with 'scroll-snap-align: end start' alignment
PASS Snaps correctly for vertical-lr writing mode with 'scroll-snap-align: end start' alignment
PASS Snaps correctly for vertical-rl writing mode with 'scroll-snap-align: end start' alignment
PASS Snaps correctly for horizontal-tb writing mode with 'scroll-snap-align: start end' alignment
PASS Snaps correctly for vertical-lr writing mode with 'scroll-snap-align: start end' alignment
PASS Snaps correctly for vertical-rl writing mode with 'scroll-snap-align: start end' alignment
PASS Snaps correctly for 'direction: rtl' with 'scroll-snap-align: end start' alignment
PASS Snaps correctly for 'direction: rtl' with 'scroll-snap-align: start end' alignment


Return to Bug 226010

Lines 1100-1108 static Ref<CSSValueList> valueForScrollSnapType(const ScrollSnapType& type) a/Source/WebCore/css/CSSComputedStyleDeclaration.cpp_sec1
- a/Source/WebCore/css/CSSComputedStyleDeclaration.cpp -3 / +3 lines
1100	static Ref<CSSValueList> valueForScrollSnapAlignment(const ScrollSnapAlign& alignment)	1100	static Ref<CSSValueList> valueForScrollSnapAlignment(const ScrollSnapAlign& alignment)
1101	{	1101	{
1102	auto value = CSSValueList::createSpaceSeparated();	1102	auto value = CSSValueList::createSpaceSeparated();
1103	value->append(CSSPrimitiveValue::create(alignment.y));	1103	value->append(CSSPrimitiveValue::create(alignment.blockAlign));
1104	if (alignment.x != alignment.y)	1104	if (alignment.inlineAlign != alignment.blockAlign)
1105	value->append(CSSPrimitiveValue::create(alignment.x));	1105	value->append(CSSPrimitiveValue::create(alignment.inlineAlign));
1106	return value;	1106	return value;
1107	}	1107	}
1108		1108

Lines 198-204 static LayoutUnit computeScrollSnapAlignOffset(LayoutUnit minLocation, LayoutUni a/Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp_sec1
- a/Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp -11 / +24 lines
198	}	198	}
199	}	199	}
200		200
201	void updateSnapOffsetsForScrollableArea(ScrollableArea& scrollableArea, const RenderBox& scrollingElementBox, const RenderStyle& scrollingElementStyle, LayoutRect viewportRectInBorderBoxCoordinates)	201	void updateSnapOffsetsForScrollableArea(ScrollableArea& scrollableArea, const RenderBox& scrollingElementBox, const RenderStyle& scrollingElementStyle, LayoutRect viewportRectInBorderBoxCoordinates, WritingMode writingMode, TextDirection textDirection)
202	{	202	{
203	auto scrollSnapType = scrollingElementStyle.scrollSnapType();	203	auto scrollSnapType = scrollingElementStyle.scrollSnapType();
204	const auto& boxesWithScrollSnapPositions = scrollingElementBox.view().boxesWithScrollSnapPositions();	204	const auto& boxesWithScrollSnapPositions = scrollingElementBox.view().boxesWithScrollSnapPositions();
Lines 218-229 void updateSnapOffsetsForScrollableArea(ScrollableArea& scrollableArea, const Re a/Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp_sec2
218	HashMap<float, SnapOffset<LayoutUnit>> verticalSnapOffsetsMap;	218	HashMap<float, SnapOffset<LayoutUnit>> verticalSnapOffsetsMap;
219	HashMap<float, SnapOffset<LayoutUnit>> horizontalSnapOffsetsMap;	219	HashMap<float, SnapOffset<LayoutUnit>> horizontalSnapOffsetsMap;
220	Vector<LayoutRect> snapAreas;	220	Vector<LayoutRect> snapAreas;
221	bool hasHorizontalSnapOffsets = scrollSnapType.axis == ScrollSnapAxis::Both \|\| scrollSnapType.axis == ScrollSnapAxis::XAxis \|\| scrollSnapType.axis == ScrollSnapAxis::Inline;
222	bool hasVerticalSnapOffsets = scrollSnapType.axis == ScrollSnapAxis::Both \|\| scrollSnapType.axis == ScrollSnapAxis::YAxis \|\| scrollSnapType.axis == ScrollSnapAxis::Block;
223		221
224	auto maxScrollOffset = scrollableArea.maximumScrollOffset();	222	auto maxScrollOffset = scrollableArea.maximumScrollOffset();
225	auto scrollPosition = LayoutPoint { scrollableArea.scrollPosition() };	223	auto scrollPosition = LayoutPoint { scrollableArea.scrollPosition() };
226	bool scrollerIsRTL = !scrollingElementBox.style().isLeftToRightDirection();	224	bool scrollerIsRTL = textDirection == TextDirection::RTL \|\| writingMode == WritingMode::RightToLeft;
		225	bool scrollerHasVerticalWritingMode = isVerticalWritingMode(writingMode);
		226
		227	bool hasHorizontalSnapOffsets = scrollSnapType.axis == ScrollSnapAxis::Both \|\| scrollSnapType.axis == ScrollSnapAxis::XAxis;
		228	bool hasVerticalSnapOffsets = scrollSnapType.axis == ScrollSnapAxis::Both \|\| scrollSnapType.axis == ScrollSnapAxis::YAxis;
		229	if (scrollSnapType.axis == ScrollSnapAxis::Block) {
		230	hasHorizontalSnapOffsets = scrollerHasVerticalWritingMode;
		231	hasVerticalSnapOffsets = !scrollerHasVerticalWritingMode;
		232	}
		233	if (scrollSnapType.axis == ScrollSnapAxis::Inline) {
		234	hasHorizontalSnapOffsets = !scrollerHasVerticalWritingMode;
		235	hasVerticalSnapOffsets = scrollerHasVerticalWritingMode;
		236	}
227		237
228	// The bounds of the scrolling container's snap port, where the top left of the scrolling container's border box is the origin.	238	// The bounds of the scrolling container's snap port, where the top left of the scrolling container's border box is the origin.
229	auto scrollSnapPort = computeScrollSnapPortOrAreaRect(viewportRectInBorderBoxCoordinates, scrollingElementStyle.scrollPadding(), InsetOrOutset::Inset);	239	auto scrollSnapPort = computeScrollSnapPortOrAreaRect(viewportRectInBorderBoxCoordinates, scrollingElementStyle.scrollPadding(), InsetOrOutset::Inset);
Lines 247-254 void updateSnapOffsetsForScrollableArea(ScrollableArea& scrollableArea, const Re a/Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp_sec3
247	auto alignment = child->style().scrollSnapAlign();	257	auto alignment = child->style().scrollSnapAlign();
248	auto stop = child->style().scrollSnapStop();	258	auto stop = child->style().scrollSnapStop();
249		259
250	bool snapsHorizontally = hasHorizontalSnapOffsets && alignment.x != ScrollSnapAxisAlignType::None;	260	ScrollSnapAxisAlignType xAlign = !scrollerHasVerticalWritingMode ? alignment.inlineAlign : alignment.blockAlign;
251	bool snapsVertically = hasVerticalSnapOffsets && alignment.y != ScrollSnapAxisAlignType::None;	261	ScrollSnapAxisAlignType yAlign = !scrollerHasVerticalWritingMode ? alignment.blockAlign : alignment.inlineAlign;
		262	bool snapsHorizontally = hasHorizontalSnapOffsets && xAlign != ScrollSnapAxisAlignType::None;
		263	bool snapsVertically = hasVerticalSnapOffsets && yAlign != ScrollSnapAxisAlignType::None;
		264
252	if (!snapsHorizontally && !snapsVertically)	265	if (!snapsHorizontally && !snapsVertically)
253	continue;	266	continue;
254		267
Lines 258-275 void updateSnapOffsetsForScrollableArea(ScrollableArea& scrollableArea, const Re a/Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp_sec4
258	snapAreas.append(scrollSnapAreaAsOffsets);	271	snapAreas.append(scrollSnapAreaAsOffsets);
259		272
260	if (snapsHorizontally) {	273	if (snapsHorizontally) {
261	auto absoluteScrollXPosition = computeScrollSnapAlignOffset(scrollSnapArea.x(), scrollSnapArea.maxX(), alignment.x, scrollerIsRTL) - computeScrollSnapAlignOffset(scrollSnapPort.x(), scrollSnapPort.maxX(), alignment.x, scrollerIsRTL);	274	auto absoluteScrollXPosition = computeScrollSnapAlignOffset(scrollSnapArea.x(), scrollSnapArea.maxX(), xAlign, scrollerIsRTL) - computeScrollSnapAlignOffset(scrollSnapPort.x(), scrollSnapPort.maxX(), xAlign, scrollerIsRTL);
262	auto absoluteScrollOffset = clampTo<int>(scrollableArea.scrollOffsetFromPosition({ roundToInt(absoluteScrollXPosition), 0 }).x(), 0, maxScrollOffset.x());	275	auto absoluteScrollOffset = clampTo<int>(scrollableArea.scrollOffsetFromPosition({ roundToInt(absoluteScrollXPosition), 0 }).x(), 0, maxScrollOffset.x());
263	addOrUpdateStopForSnapOffset(horizontalSnapOffsetsMap, { absoluteScrollOffset, stop, snapAreas.size() - 1 });	276	addOrUpdateStopForSnapOffset(horizontalSnapOffsetsMap, { absoluteScrollOffset, stop, snapAreas.size() - 1 });
264	}	277	}
265	if (snapsVertically) {	278	if (snapsVertically) {
266	auto absoluteScrollYPosition = computeScrollSnapAlignOffset(scrollSnapArea.y(), scrollSnapArea.maxY(), alignment.y, false) - computeScrollSnapAlignOffset(scrollSnapPort.y(), scrollSnapPort.maxY(), alignment.y, false);	279	auto absoluteScrollYPosition = computeScrollSnapAlignOffset(scrollSnapArea.y(), scrollSnapArea.maxY(), yAlign, false) - computeScrollSnapAlignOffset(scrollSnapPort.y(), scrollSnapPort.maxY(), yAlign, false);
267	auto absoluteScrollOffset = clampTo<int>(scrollableArea.scrollOffsetFromPosition({ 0, roundToInt(absoluteScrollYPosition) }).y(), 0, maxScrollOffset.y());	280	auto absoluteScrollOffset = clampTo<int>(scrollableArea.scrollOffsetFromPosition({ 0, roundToInt(absoluteScrollYPosition) }).y(), 0, maxScrollOffset.y());
268	addOrUpdateStopForSnapOffset(verticalSnapOffsetsMap, { absoluteScrollOffset, stop, snapAreas.size() - 1 });	281	addOrUpdateStopForSnapOffset(verticalSnapOffsetsMap, { absoluteScrollOffset, stop, snapAreas.size() - 1 });
269	}	282	}
270
271	if (!snapAreas.isEmpty())
272	LOG_WITH_STREAM(ScrollSnap, stream << " => Computed snap areas: " << snapAreas);
273	}	283	}
274		284
275	auto compareSnapOffsets = [](const SnapOffset<LayoutUnit>& a, const SnapOffset<LayoutUnit>& b)	285	auto compareSnapOffsets = [](const SnapOffset<LayoutUnit>& a, const SnapOffset<LayoutUnit>& b)
Lines 277-282 void updateSnapOffsetsForScrollableArea(ScrollableArea& scrollableArea, const Re a/Source/WebCore/page/scrolling/ScrollSnapOffsetsInfo.cpp_sec5
277	return a.offset < b.offset;	287	return a.offset < b.offset;
278	};	288	};
279		289
		290	if (!snapAreas.isEmpty())
		291	LOG_WITH_STREAM(ScrollSnap, stream << " => Computed snap areas: " << snapAreas);
		292
280	Vector<SnapOffset<LayoutUnit>> horizontalSnapOffsets = copyToVector(horizontalSnapOffsetsMap.values());	293	Vector<SnapOffset<LayoutUnit>> horizontalSnapOffsets = copyToVector(horizontalSnapOffsetsMap.values());
281	if (!horizontalSnapOffsets.isEmpty()) {	294	if (!horizontalSnapOffsets.isEmpty()) {
282	std::sort(horizontalSnapOffsets.begin(), horizontalSnapOffsets.end(), compareSnapOffsets);	295	std::sort(horizontalSnapOffsets.begin(), horizontalSnapOffsets.end(), compareSnapOffsets);

Lines 1469-1475 void RenderLayerScrollableArea::updateSnapOffsets() a/Source/WebCore/rendering/RenderLayerScrollableArea.cpp_sec1
- a/Source/WebCore/rendering/RenderLayerScrollableArea.cpp -1 / +2 lines
1469	return;	1469	return;
1470		1470
1471	RenderBox* box = m_layer.enclosingElement()->renderBox();	1471	RenderBox* box = m_layer.enclosingElement()->renderBox();
1472	updateSnapOffsetsForScrollableArea(this, box, box->style(), box->paddingBoxRect());	1472	fprintf(stderr, "vertical: %i\n", box->style().isVerticalWritingMode());
		1473	updateSnapOffsetsForScrollableArea(this, box, box->style(), box->paddingBoxRect(), box->style().writingMode(), box->style().direction());
1473	}	1474	}
1474		1475
1475	bool RenderLayerScrollableArea::isScrollSnapInProgress() const	1476	bool RenderLayerScrollableArea::isScrollSnapInProgress() const

Lines 920-930 inline ScrollSnapAlign BuilderConverter::convertScrollSnapAlign(BuilderState&, c a/Source/WebCore/style/StyleBuilderConverter.h_sec1
- a/Source/WebCore/style/StyleBuilderConverter.h -3 / +3 lines
920	{	920	{
921	auto& values = downcast<CSSValueList>(value);	921	auto& values = downcast<CSSValueList>(value);
922	ScrollSnapAlign alignment;	922	ScrollSnapAlign alignment;
923	alignment.y = downcast<CSSPrimitiveValue>(*values.item(0));	923	alignment.blockAlign = downcast<CSSPrimitiveValue>(*values.item(0));
924	if (values.length() == 1)	924	if (values.length() == 1)
925	alignment.x = alignment.y;	925	alignment.inlineAlign = alignment.blockAlign;
926	else	926	else
927	alignment.x = downcast<CSSPrimitiveValue>(*values.item(1));	927	alignment.inlineAlign = downcast<CSSPrimitiveValue>(*values.item(1));
928	return alignment;	928	return alignment;
929	}	929	}
930		930

Lines 1-3 a/Source/WebCore/ChangeLog_sec1
- a/Source/WebCore/ChangeLog +39 lines
		1	2021-05-20 Martin Robinson <mrobinson@igalia.com>
		2
		3	[css-scroll-snap] Scroll snap is broken with non-horizontal writing modes
		4	https://bugs.webkit.org/show_bug.cgi?id=226010
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	Fix issues related to vertical writing modes and scroll snap.
		9
		10	This change fixes three existing WPT tests.
		11	imported/w3c/web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element.html
		12	imported/w3c/web-platform-tests/css/css-scroll-snap/snap-after-initial-layout/writing-mode-vertical-lr.html
		13	imported/w3c/web-platform-tests/css/css-scroll-snap/snap-inline-block.html
		14
		15	* css/CSSComputedStyleDeclaration.cpp:
		16	(WebCore::valueForScrollSnapAlignment): Update to reflect new member names of ScrollSnapAlign.
		17	* page/FrameView.cpp:
		18	(WebCore::FrameView::updateSnapOffsets): Pass in the text direction and writing mode of the
		19	container, allowing the values specified on the body to override those specified on the root
		20	element.
		21	* page/scrolling/ScrollSnapOffsetsInfo.cpp:
		22	(WebCore::updateSnapOffsetsForScrollableArea): Properly handle the writing mode and the
		23	text direction of the container.
		24	* page/scrolling/ScrollSnapOffsetsInfo.h: Update function signature.
		25	* rendering/RenderLayerModelObject.cpp:
		26	(WebCore::scrollSnapContainerRequiresUpdateForStyleUpdate): Pass in the writing mode and
		27	text direction of the scrolling container.
		28	* rendering/RenderLayerScrollableArea.cpp:
		29	(WebCore::RenderLayerScrollableArea::updateSnapOffsets): Update to reflect new member names
		30	of ScrollSnapAlign.
		31	* rendering/style/RenderStyle.cpp:
		32	(WebCore::RenderStyle::hasSnapPosition const): Ditto.
		33	* rendering/style/StyleScrollSnapPoints.h: Change the name of the members of ScrollSnapAlign
		34	to match what is described in the specification. The values provided are for block and
		35	inline directions, but depending on the scroll container.
		36	(WebCore::operator==): Ditto.
		37	* style/StyleBuilderConverter.h:
		38	(WebCore::Style::BuilderConverter::convertScrollSnapAlign): Ditto.
		39
1	2021-05-18 Cameron McCormack <heycam@apple.com>	40	2021-05-18 Cameron McCormack <heycam@apple.com>
2		41
3	Record gradient and pattern filled canvas text in the correct coordinate system.	42	Record gradient and pattern filled canvas text in the correct coordinate system.

Lines 48-60 inline bool operator==(const ScrollSnapType& a, const ScrollSnapType& b) a/Source/WebCore/rendering/style/StyleScrollSnapPoints.h_sec1
- a/Source/WebCore/rendering/style/StyleScrollSnapPoints.h -3 / +3 lines
48	inline bool operator!=(const ScrollSnapType& a, const ScrollSnapType& b) { return !(a == b); }	48	inline bool operator!=(const ScrollSnapType& a, const ScrollSnapType& b) { return !(a == b); }
49		49
50	struct ScrollSnapAlign {	50	struct ScrollSnapAlign {
51	ScrollSnapAxisAlignType x { ScrollSnapAxisAlignType::None };	51	ScrollSnapAxisAlignType blockAlign { ScrollSnapAxisAlignType::None };
52	ScrollSnapAxisAlignType y { ScrollSnapAxisAlignType::None };	52	ScrollSnapAxisAlignType inlineAlign { ScrollSnapAxisAlignType::None };
53	};	53	};
54		54
55	inline bool operator==(const ScrollSnapAlign& a, const ScrollSnapAlign& b)	55	inline bool operator==(const ScrollSnapAlign& a, const ScrollSnapAlign& b)
56	{	56	{
57	return a.x == b.x && a.y == b.y;	57	return a.blockAlign == b.blockAlign && a.inlineAlign == b.inlineAlign;
58	}	58	}
59		59
60	inline bool operator!=(const ScrollSnapAlign& a, const ScrollSnapAlign& b) { return !(a == b); }	60	inline bool operator!=(const ScrollSnapAlign& a, const ScrollSnapAlign& b) { return !(a == b); }

Lines 1-3 a/LayoutTests/ChangeLog_sec1
- a/LayoutTests/ChangeLog +9 lines
		1	2021-05-20 Martin Robinson <mrobinson@igalia.com>
		2
		3	[css-scroll-snap] Scroll snap is broken with non-horizontal writing modes
		4	https://bugs.webkit.org/show_bug.cgi?id=226010
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	* TestExpectations: Mark one test as passing.
		9
1	2021-05-18 Cameron McCormack <heycam@apple.com>	10	2021-05-18 Cameron McCormack <heycam@apple.com>
2		11
3	Record gradient and pattern filled canvas text in the correct coordinate system.	12	Record gradient and pattern filled canvas text in the correct coordinate system.

Lines 1-3 a/LayoutTests/imported/w3c/ChangeLog_sec1
- a/LayoutTests/imported/w3c/ChangeLog +10 lines
		1	2021-05-20 Martin Robinson <mrobinson@igalia.com>
		2
		3	[css-scroll-snap] Scroll snap is broken with non-horizontal writing modes
		4	https://bugs.webkit.org/show_bug.cgi?id=226010
		5
		6	Reviewed by NOBODY (OOPS!).
		7
		8	* web-platform-tests/css/css-scroll-snap/scroll-snap-type-on-root-element-expected.txt: Update expectations to mark tests as passing.
		9	* web-platform-tests/css/css-scroll-snap/snap-inline-block-expected.txt: Ditto.
		10
1	2021-05-18 Keith Miller <keith_miller@apple.com>	11	2021-05-18 Keith Miller <keith_miller@apple.com>
2		12
3	Temporarily revert r276592 as it breaks some native apps	13	Temporarily revert r276592 as it breaks some native apps

Lines 1-10 a/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-inline-block-expected.txt_sec1
- a/LayoutTests/imported/w3c/web-platform-tests/css/css-scroll-snap/snap-inline-block-expected.txt -7 / +7 lines
1		1
2	PASS Snaps correctly for horizontal-tb writing mode with 'scroll-snap-align: end start' alignment	2	PASS Snaps correctly for horizontal-tb writing mode with 'scroll-snap-align: end start' alignment
3	FAIL Snaps correctly for vertical-lr writing mode with 'scroll-snap-align: end start' alignment assert_equals: aligns correctly on x expected 115 but got 300	3	PASS Snaps correctly for vertical-lr writing mode with 'scroll-snap-align: end start' alignment
4	FAIL Snaps correctly for vertical-rl writing mode with 'scroll-snap-align: end start' alignment assert_equals: aligns correctly on y expected 300 but got 165	4	PASS Snaps correctly for vertical-rl writing mode with 'scroll-snap-align: end start' alignment
5	FAIL Snaps correctly for horizontal-tb writing mode with 'scroll-snap-align: start end' alignment assert_equals: aligns correctly on x expected 115 but got 0	5	PASS Snaps correctly for horizontal-tb writing mode with 'scroll-snap-align: start end' alignment
6	FAIL Snaps correctly for vertical-lr writing mode with 'scroll-snap-align: start end' alignment assert_equals: aligns correctly on x expected 300 but got 0	6	PASS Snaps correctly for vertical-lr writing mode with 'scroll-snap-align: start end' alignment
7	FAIL Snaps correctly for vertical-rl writing mode with 'scroll-snap-align: start end' alignment assert_equals: aligns correctly on y expected 165 but got 300	7	PASS Snaps correctly for vertical-rl writing mode with 'scroll-snap-align: start end' alignment
8	FAIL Snaps correctly for 'direction: rtl' with 'scroll-snap-align: end start' alignment assert_equals: aligns correctly on y expected 165 but got 0	8	PASS Snaps correctly for 'direction: rtl' with 'scroll-snap-align: end start' alignment
9	FAIL Snaps correctly for 'direction: rtl' with 'scroll-snap-align: start end' alignment assert_equals: aligns correctly on y expected 300 but got 0	9	PASS Snaps correctly for 'direction: rtl' with 'scroll-snap-align: start end' alignment
10		10