- Revision
- 281185
- Author
- mrobin...@webkit.org
- Date
- 2021-08-18 06:30:04 -0700 (Wed, 18 Aug 2021)
Log Message
position: sticky with display: inline-block
https://bugs.webkit.org/show_bug.cgi?id=224415
<rdar://problem/76811968>
Reviewed by Simon Fraser.
Source/WebCore:
Improve sticky positioning applied to inline items. The first improvement is to
skip anonymous RenderBlock parents of inline display items when looking for the
containing block used to calculate sticky constraints. These anonymous parents
are not the containing block that constrains the movement of these kind of stickily
positioned items. Instead look for containing block for the anonymous parent,
which does limit the scroll boundaries of inline stickily position items.
Previously, when converting the frame rect from the coordinate space of the stickily
positioned item to that of the scrolling container, the code used to apply the offset
from the containing block to the scrolling ancestor directly to the frame rect. That
doesn't work when the containing block that determines sticky constraints is not the
direct containing block of the stickily positioned items (as it is for inlines).
Instead, the code now just maps the frame rect directly from the item parent to
the scrolling ancestor. This simplifies things a bit and allows it to work with
inline stickily positioned items.
Finally, this change adds comments to make it clearer what this method is doing.
No new tests. This is covered by existing WPT tests. Specifically the following
tests are now passing:
imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-hyperlink.html
imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-nested-table.html
imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-table-parts.html
imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-table-th-bottom.html
imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-nested-thead-th.html
* css/CSSComputedStyleDeclaration.cpp:
(WebCore::positionOffsetValue): enclosingClippingBoxForStickyPosition now returns a pair
to avoid passing in a pointer. Only look at the first part of the pair here.
* rendering/RenderBoxModelObject.cpp:
(WebCore::RenderBoxModelObject::computeStickyPositionConstraints const): Modified this method
to return a pair in order to avoid dealing with input parameters. This allows simplifying this
method, making the code a bit simpler.
(WebCore::RenderBoxModelObject::enclosingClippingBoxForStickyPosition const): Deleted.
(WebCore::RenderBoxModelObject::computeStickyPositionConstraints const): Make two changes
to this method to improve sticky positioning for inline elements. The first change is
to be a bit better about finding the appropriate containing block for calculating sticky
constraints. The second is to more resiliently move from coordinate systems. In addition,
improve the comments for this method to make it clearer what is happening at each step.
* rendering/RenderBoxModelObject.h: Updated method declaration.
LayoutTests:
* TestExpectations: Unskip newly passing tests and skip one test which was
a false pass previously.
Modified Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (281184 => 281185)
--- trunk/LayoutTests/ChangeLog 2021-08-18 09:13:47 UTC (rev 281184)
+++ trunk/LayoutTests/ChangeLog 2021-08-18 13:30:04 UTC (rev 281185)
@@ -1,3 +1,14 @@
+2021-08-18 Martin Robinson <mrobin...@webkit.org>
+
+ position: sticky with display: inline-block
+ https://bugs.webkit.org/show_bug.cgi?id=224415
+ <rdar://problem/76811968>
+
+ Reviewed by Simon Fraser.
+
+ * TestExpectations: Unskip newly passing tests and skip one test which was
+ a false pass previously.
+
2021-08-17 Alex Christensen <achristen...@webkit.org>
Mark test as flaky: imported/w3c/web-platform-tests/navigation-timing/nav2_test_attributes_values.html
Modified: trunk/LayoutTests/TestExpectations (281184 => 281185)
--- trunk/LayoutTests/TestExpectations 2021-08-18 09:13:47 UTC (rev 281184)
+++ trunk/LayoutTests/TestExpectations 2021-08-18 13:30:04 UTC (rev 281185)
@@ -3482,17 +3482,13 @@
webkit.org/b/228993 imported/w3c/web-platform-tests/css/css-position/multicol/static-position/vrl-ltr-rtl-in-multicol.tentative.html [ ImageOnlyFailure ]
webkit.org/b/228993 imported/w3c/web-platform-tests/css/css-position/multicol/static-position/vrl-rtl-ltr-in-multicol.tentative.html [ ImageOnlyFailure ]
webkit.org/b/228993 imported/w3c/web-platform-tests/css/css-position/multicol/static-position/vrl-rtl-rtl-in-multicol.html [ ImageOnlyFailure ]
-webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-hyperlink.html [ ImageOnlyFailure ]
webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-inline.html [ ImageOnlyFailure ]
webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-nested-inline.html [ ImageOnlyFailure ]
-webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-nested-table.html [ ImageOnlyFailure ]
webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-rendering.html [ ImageOnlyFailure ]
-webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-table-parts.html [ ImageOnlyFailure ]
-webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-table-th-bottom.html [ ImageOnlyFailure ]
webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-writing-modes.html [ ImageOnlyFailure ]
webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-large-top-2.tentative.html [ ImageOnlyFailure ]
webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-large-top.tentative.html [ ImageOnlyFailure ]
-webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-nested-thead-th.html [ ImageOnlyFailure ]
+webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/sticky-after-input.html [ Failure ]
webkit.org/b/203451 imported/w3c/web-platform-tests/css/css-position/static-position/htb-ltr-ltr.html [ ImageOnlyFailure ]
webkit.org/b/203451 imported/w3c/web-platform-tests/css/css-position/static-position/htb-ltr-rtl.tentative.html [ ImageOnlyFailure ]
webkit.org/b/203451 imported/w3c/web-platform-tests/css/css-position/static-position/htb-rtl-ltr.tentative.html [ ImageOnlyFailure ]
Modified: trunk/Source/WebCore/ChangeLog (281184 => 281185)
--- trunk/Source/WebCore/ChangeLog 2021-08-18 09:13:47 UTC (rev 281184)
+++ trunk/Source/WebCore/ChangeLog 2021-08-18 13:30:04 UTC (rev 281185)
@@ -1,3 +1,53 @@
+2021-08-18 Martin Robinson <mrobin...@webkit.org>
+
+ position: sticky with display: inline-block
+ https://bugs.webkit.org/show_bug.cgi?id=224415
+ <rdar://problem/76811968>
+
+ Reviewed by Simon Fraser.
+
+ Improve sticky positioning applied to inline items. The first improvement is to
+ skip anonymous RenderBlock parents of inline display items when looking for the
+ containing block used to calculate sticky constraints. These anonymous parents
+ are not the containing block that constrains the movement of these kind of stickily
+ positioned items. Instead look for containing block for the anonymous parent,
+ which does limit the scroll boundaries of inline stickily position items.
+
+ Previously, when converting the frame rect from the coordinate space of the stickily
+ positioned item to that of the scrolling container, the code used to apply the offset
+ from the containing block to the scrolling ancestor directly to the frame rect. That
+ doesn't work when the containing block that determines sticky constraints is not the
+ direct containing block of the stickily positioned items (as it is for inlines).
+
+ Instead, the code now just maps the frame rect directly from the item parent to
+ the scrolling ancestor. This simplifies things a bit and allows it to work with
+ inline stickily positioned items.
+
+ Finally, this change adds comments to make it clearer what this method is doing.
+
+ No new tests. This is covered by existing WPT tests. Specifically the following
+ tests are now passing:
+ imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-hyperlink.html
+ imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-nested-table.html
+ imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-table-parts.html
+ imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-table-th-bottom.html
+ imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-nested-thead-th.html
+
+ * css/CSSComputedStyleDeclaration.cpp:
+ (WebCore::positionOffsetValue): enclosingClippingBoxForStickyPosition now returns a pair
+ to avoid passing in a pointer. Only look at the first part of the pair here.
+ * rendering/RenderBoxModelObject.cpp:
+ (WebCore::RenderBoxModelObject::computeStickyPositionConstraints const): Modified this method
+ to return a pair in order to avoid dealing with input parameters. This allows simplifying this
+ method, making the code a bit simpler.
+ (WebCore::RenderBoxModelObject::enclosingClippingBoxForStickyPosition const): Deleted.
+ (WebCore::RenderBoxModelObject::computeStickyPositionConstraints const): Make two changes
+ to this method to improve sticky positioning for inline elements. The first change is
+ to be a bit better about finding the appropriate containing block for calculating sticky
+ constraints. The second is to more resiliently move from coordinate systems. In addition,
+ improve the comments for this method to make it clearer what is happening at each step.
+ * rendering/RenderBoxModelObject.h: Updated method declaration.
+
2021-08-18 Xabier Rodriguez Calvar <calva...@igalia.com>
[GStreamer][EME] Try to parse XML init datas that could possibly come from MPD manifests
Modified: trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp (281184 => 281185)
--- trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp 2021-08-18 09:13:47 UTC (rev 281184)
+++ trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp 2021-08-18 13:30:04 UTC (rev 281185)
@@ -399,7 +399,7 @@
}
LayoutUnit containingBlockSize;
if (box.isStickilyPositioned()) {
- auto& enclosingClippingBox = box.enclosingClippingBoxForStickyPosition();
+ auto& enclosingClippingBox = box.enclosingClippingBoxForStickyPosition().first;
if (isVerticalProperty == enclosingClippingBox.isHorizontalWritingMode())
containingBlockSize = enclosingClippingBox.contentLogicalHeight();
else
Modified: trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp (281184 => 281185)
--- trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2021-08-18 09:13:47 UTC (rev 281184)
+++ trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp 2021-08-18 13:30:04 UTC (rev 281185)
@@ -440,16 +440,12 @@
return referencePoint;
}
-const RenderBox& RenderBoxModelObject::enclosingClippingBoxForStickyPosition(const RenderLayer** enclosingClippingLayer) const
+std::pair<const RenderBox&, const RenderLayer*> RenderBoxModelObject::enclosingClippingBoxForStickyPosition() const
{
ASSERT(isStickilyPositioned());
-
RenderLayer* clipLayer = hasLayer() ? layer()->enclosingOverflowClipLayer(ExcludeSelf) : nullptr;
-
- if (enclosingClippingLayer)
- *enclosingClippingLayer = clipLayer;
-
- return clipLayer ? downcast<RenderBox>(clipLayer->renderer()) : view();
+ const RenderBox& box = clipLayer ? downcast<RenderBox>(clipLayer->renderer()) : view();
+ return { box, clipLayer };
}
void RenderBoxModelObject::computeStickyPositionConstraints(StickyPositionViewportConstraints& constraints, const FloatRect& constrainingRect) const
@@ -456,14 +452,24 @@
{
constraints.setConstrainingRectAtLastLayout(constrainingRect);
+ // Do not use anonymous containing blocks to determine sticky constraints. We want the size
+ // of the first true containing block, because that is what imposes the limitation on the
+ // movement of stickily positioned items.
RenderBlock* containingBlock = this->containingBlock();
- const RenderLayer* enclosingClippingLayer = nullptr;
- auto& enclosingClippingBox = enclosingClippingBoxForStickyPosition(&enclosingClippingLayer);
+ while (containingBlock && (!is<RenderBlock>(*containingBlock) || containingBlock->isAnonymousBlock()))
+ containingBlock = containingBlock->containingBlock();
+ ASSERT(containingBlock);
+ auto [enclosingClippingBox, enclosingClippingLayer] = enclosingClippingBoxForStickyPosition();
+
LayoutRect containerContentRect;
- if (!enclosingClippingLayer || (containingBlock != &enclosingClippingBox))
+ if (!enclosingClippingLayer || (containingBlock != &enclosingClippingBox)) {
+ // In this case either the scrolling element is the view or there is another containing block in
+ // the hierarchy between this stickily positioned item and its scrolling ancestor. In both cases,
+ // we use the content box rectangle of the containing block, which is what should constrain the
+ // movement.
containerContentRect = containingBlock->contentBoxRect();
- else {
+ } else {
containerContentRect = containingBlock->layoutOverflowRect();
LayoutPoint containerLocation = containerContentRect.location() + LayoutPoint(containingBlock->borderLeft() + containingBlock->paddingLeft(),
containingBlock->borderTop() + containingBlock->paddingTop());
@@ -498,24 +504,30 @@
// Now compute the sticky box rect, also relative to the scrolling ancestor.
LayoutRect stickyBoxRect = frameRectForStickyPositioning();
- LayoutRect flippedStickyBoxRect = stickyBoxRect;
- containingBlock->flipForWritingMode(flippedStickyBoxRect);
- FloatRect stickyBoxRelativeToScrollingAnecstor = flippedStickyBoxRect;
- // FIXME: sucks to call localToContainerQuad again, but we can't just offset from the previously computed rect if there are transforms.
- // Map to the view to avoid including page scale factor.
- FloatPoint stickyLocationRelativeToScrollingAncestor = flippedStickyBoxRect.location() + containingBlock->localToContainerQuad(FloatRect(FloatPoint(), containingBlock->size()), &enclosingClippingBox).boundingBox().location();
+ // Ideally, it would be possible to call this->localToContainerQuad to determine the frame
+ // rectangle in the coordinate system of the scrolling ancestor, but localToContainerQuad
+ // itself depends on sticky positioning! Instead, start from the parent but first adjusting
+ // the rectangle for the writing mode of this stickily-positioned element.
+ //
+ // FIXME: For now, assume that |this| is not transformed. It would also be nice to not have to
+ // call localToContainerQuad again since we have already done a similar call to move from
+ // the containing block to the scrolling ancestor above, but localToContainerQuad takes care
+ // of a lot of complex situations involving inlines, tables, and transformations.
+ if (parent()->isBox())
+ downcast<RenderBox>(parent())->flipForWritingMode(stickyBoxRect);
+ auto stickyBoxRelativeToScrollingAncestor = parent()->localToContainerQuad(FloatRect(stickyBoxRect), &enclosingClippingBox).boundingBox();
+
if (enclosingClippingLayer) {
- stickyLocationRelativeToScrollingAncestor -= FloatSize(enclosingClippingBox.borderLeft() + enclosingClippingBox.paddingLeft(),
- enclosingClippingBox.borderTop() + enclosingClippingBox.paddingTop());
- if (&enclosingClippingBox != containingBlock) {
+ stickyBoxRelativeToScrollingAncestor.move(-FloatSize(enclosingClippingBox.borderLeft() + enclosingClippingBox.paddingLeft(),
+ enclosingClippingBox.borderTop() + enclosingClippingBox.paddingTop()));
+
+ if (&enclosingClippingBox != parent()) {
if (auto* scrollableArea = enclosingClippingLayer->scrollableArea())
- stickyLocationRelativeToScrollingAncestor += scrollableArea->scrollOffset();
+ stickyBoxRelativeToScrollingAncestor.moveBy(scrollableArea->scrollOffset());
}
}
- // FIXME: For now, assume that |this| is not transformed.
- stickyBoxRelativeToScrollingAnecstor.setLocation(stickyLocationRelativeToScrollingAncestor);
- constraints.setStickyBoxRect(stickyBoxRelativeToScrollingAnecstor);
+ constraints.setStickyBoxRect(stickyBoxRelativeToScrollingAncestor);
if (!style().left().isAuto()) {
constraints.setLeftOffset(valueForLength(style().left(), constrainingRect.width()));
Modified: trunk/Source/WebCore/rendering/RenderBoxModelObject.h (281184 => 281185)
--- trunk/Source/WebCore/rendering/RenderBoxModelObject.h 2021-08-18 09:13:47 UTC (rev 281184)
+++ trunk/Source/WebCore/rendering/RenderBoxModelObject.h 2021-08-18 13:30:04 UTC (rev 281185)
@@ -116,7 +116,7 @@
LayoutSize relativePositionLogicalOffset() const { return style().isHorizontalWritingMode() ? relativePositionOffset() : relativePositionOffset().transposedSize(); }
FloatRect constrainingRectForStickyPosition() const;
- const RenderBox& enclosingClippingBoxForStickyPosition(const RenderLayer** enclosingClippingLayer = nullptr) const;
+ std::pair<const RenderBox&, const RenderLayer*> enclosingClippingBoxForStickyPosition() const;
void computeStickyPositionConstraints(StickyPositionViewportConstraints&, const FloatRect& constrainingRect) const;
LayoutSize stickyPositionOffset() const;
LayoutSize stickyPositionLogicalOffset() const { return style().isHorizontalWritingMode() ? stickyPositionOffset() : stickyPositionOffset().transposedSize(); }
