⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 281185 in webkit


Ignore:
Timestamp:
Aug 18, 2021, 6:30:04 AM (5 years ago)
Author:
Martin Robinson
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:

a false pass previously.

Location:
trunk
Files:
6 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r281181 r281185  
     12021-08-18  Martin Robinson  <mrobinson@webkit.org>
     2
     3        position: sticky with display: inline-block
     4        https://bugs.webkit.org/show_bug.cgi?id=224415
     5        <rdar://problem/76811968>
     6
     7        Reviewed by Simon Fraser.
     8
     9        * TestExpectations: Unskip newly passing tests and skip one test which was
     10        a false pass previously.
     11
    1122021-08-17  Alex Christensen  <achristensen@webkit.org>
    213

  • trunk/LayoutTests/TestExpectations

    r281181 r281185  
    34833483webkit.org/b/228993 imported/w3c/web-platform-tests/css/css-position/multicol/static-position/vrl-rtl-ltr-in-multicol.tentative.html [ ImageOnlyFailure ]
    34843484webkit.org/b/228993 imported/w3c/web-platform-tests/css/css-position/multicol/static-position/vrl-rtl-rtl-in-multicol.html [ ImageOnlyFailure ]
    3485 webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-hyperlink.html [ ImageOnlyFailure ]
    34863485webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-inline.html [ ImageOnlyFailure ]
    34873486webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-nested-inline.html [ ImageOnlyFailure ]
    3488 webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-nested-table.html [ ImageOnlyFailure ]
    34893487webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-rendering.html [ ImageOnlyFailure ]
    3490 webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-table-parts.html [ ImageOnlyFailure ]
    3491 webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-table-th-bottom.html [ ImageOnlyFailure ]
    34923488webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-writing-modes.html [ ImageOnlyFailure ]
    34933489webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-large-top-2.tentative.html [ ImageOnlyFailure ]
    34943490webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-large-top.tentative.html [ ImageOnlyFailure ]
    3495 webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-nested-thead-th.html [ ImageOnlyFailure ]
     3491webkit.org/b/203450 imported/w3c/web-platform-tests/css/css-position/sticky/sticky-after-input.html [ Failure ]
    34963492webkit.org/b/203451 imported/w3c/web-platform-tests/css/css-position/static-position/htb-ltr-ltr.html [ ImageOnlyFailure ]
    34973493webkit.org/b/203451 imported/w3c/web-platform-tests/css/css-position/static-position/htb-ltr-rtl.tentative.html [ ImageOnlyFailure ]

  • trunk/Source/WebCore/ChangeLog

    r281183 r281185  
     12021-08-18  Martin Robinson  <mrobinson@webkit.org>
     2
     3        position: sticky with display: inline-block
     4        https://bugs.webkit.org/show_bug.cgi?id=224415
     5        <rdar://problem/76811968>
     6
     7        Reviewed by Simon Fraser.
     8
     9        Improve sticky positioning applied to inline items. The first improvement is to
     10        skip anonymous RenderBlock parents of inline display items when looking for the
     11        containing block used to calculate sticky constraints. These anonymous parents
     12        are not the containing block that constrains the movement of these kind of stickily
     13        positioned items. Instead look for containing block for the anonymous parent,
     14        which does limit the scroll boundaries of inline stickily position items.
     15
     16        Previously, when converting the frame rect from the coordinate space of the stickily
     17        positioned item to that of the scrolling container, the code used to apply the offset
     18        from the containing block to the scrolling ancestor directly to the frame rect. That
     19        doesn't work when the containing block that determines sticky constraints is not the
     20        direct containing block of the stickily positioned items (as it is for inlines).
     21
     22        Instead, the code now just maps the frame rect directly from the item parent to
     23        the scrolling ancestor. This simplifies things a bit and allows it to work with
     24        inline stickily positioned items.
     25
     26        Finally, this change adds comments to make it clearer what this method is doing.
     27
     28        No new tests. This is covered by existing WPT tests. Specifically the following
     29        tests are now passing:
     30         imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-hyperlink.html
     31         imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-nested-table.html
     32         imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-table-parts.html
     33         imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-table-th-bottom.html
     34         imported/w3c/web-platform-tests/css/css-position/sticky/position-sticky-nested-thead-th.html
     35
     36        * css/CSSComputedStyleDeclaration.cpp:
     37        (WebCore::positionOffsetValue): enclosingClippingBoxForStickyPosition now returns a pair
     38        to avoid passing in a pointer. Only look at the first part of the pair here.
     39        * rendering/RenderBoxModelObject.cpp:
     40        (WebCore::RenderBoxModelObject::computeStickyPositionConstraints const): Modified this method
     41        to return a pair in order to avoid dealing with input parameters. This allows simplifying this
     42        method, making the code a bit simpler.
     43        (WebCore::RenderBoxModelObject::enclosingClippingBoxForStickyPosition const): Deleted.
     44        (WebCore::RenderBoxModelObject::computeStickyPositionConstraints const): Make two changes
     45        to this method to improve sticky positioning for inline elements. The first change is
     46        to be a bit better about finding the appropriate containing block for calculating sticky
     47        constraints. The second is to more resiliently move from coordinate systems. In addition,
     48        improve the comments for this method to make it clearer what is happening at each step.
     49        * rendering/RenderBoxModelObject.h: Updated method declaration.
     50
    1512021-08-18  Xabier Rodriguez Calvar  <calvaris@igalia.com>
    252

  • trunk/Source/WebCore/css/CSSComputedStyleDeclaration.cpp

    r281025 r281185  
    400400        LayoutUnit containingBlockSize;
    401401        if (box.isStickilyPositioned()) {
    402             auto& enclosingClippingBox = box.enclosingClippingBoxForStickyPosition();
     402            auto& enclosingClippingBox = box.enclosingClippingBoxForStickyPosition().first;
    403403            if (isVerticalProperty == enclosingClippingBox.isHorizontalWritingMode())
    404404                containingBlockSize = enclosingClippingBox.contentLogicalHeight();

  • trunk/Source/WebCore/rendering/RenderBoxModelObject.cpp

    r279918 r281185  
    441441}
    442442
    443 const RenderBox& RenderBoxModelObject::enclosingClippingBoxForStickyPosition(const RenderLayer** enclosingClippingLayer) const
     443std::pair<const RenderBox&, const RenderLayer*> RenderBoxModelObject::enclosingClippingBoxForStickyPosition() const
    444444{
    445445    ASSERT(isStickilyPositioned());
    446 
    447446    RenderLayer* clipLayer = hasLayer() ? layer()->enclosingOverflowClipLayer(ExcludeSelf) : nullptr;
    448 
    449     if (enclosingClippingLayer)
    450         *enclosingClippingLayer = clipLayer;
    451 
    452     return clipLayer ? downcast<RenderBox>(clipLayer->renderer()) : view();
     447    const RenderBox& box = clipLayer ? downcast<RenderBox>(clipLayer->renderer()) : view();
     448    return { box, clipLayer };
    453449}
    454450
    … …  
    457453    constraints.setConstrainingRectAtLastLayout(constrainingRect);
    458454
     455    // Do not use anonymous containing blocks to determine sticky constraints. We want the size
     456    // of the first true containing block, because that is what imposes the limitation on the
     457    // movement of stickily positioned items.
    459458    RenderBlock* containingBlock = this->containingBlock();
    460     const RenderLayer* enclosingClippingLayer = nullptr;
    461     auto& enclosingClippingBox = enclosingClippingBoxForStickyPosition(&enclosingClippingLayer);
     459    while (containingBlock && (!is<RenderBlock>(*containingBlock) || containingBlock->isAnonymousBlock()))
     460        containingBlock = containingBlock->containingBlock();
     461    ASSERT(containingBlock);
     462
     463    auto [enclosingClippingBox, enclosingClippingLayer] = enclosingClippingBoxForStickyPosition();
    462464
    463465    LayoutRect containerContentRect;
    464     if (!enclosingClippingLayer || (containingBlock != &enclosingClippingBox))
     466    if (!enclosingClippingLayer || (containingBlock != &enclosingClippingBox)) {
     467        // In this case either the scrolling element is the view or there is another containing block in
     468        // the hierarchy between this stickily positioned item and its scrolling ancestor. In both cases,
     469        // we use the content box rectangle of the containing block, which is what should constrain the
     470        // movement.
    465471        containerContentRect = containingBlock->contentBoxRect();
    466     else {
     472    } else {
    467473        containerContentRect = containingBlock->layoutOverflowRect();
    468474        LayoutPoint containerLocation = containerContentRect.location() + LayoutPoint(containingBlock->borderLeft() + containingBlock->paddingLeft(),
    … …  
    499505    // Now compute the sticky box rect, also relative to the scrolling ancestor.
    500506    LayoutRect stickyBoxRect = frameRectForStickyPositioning();
    501     LayoutRect flippedStickyBoxRect = stickyBoxRect;
    502     containingBlock->flipForWritingMode(flippedStickyBoxRect);
    503     FloatRect stickyBoxRelativeToScrollingAnecstor = flippedStickyBoxRect;
    504 
    505     // FIXME: sucks to call localToContainerQuad again, but we can't just offset from the previously computed rect if there are transforms.
    506     // Map to the view to avoid including page scale factor.
    507     FloatPoint stickyLocationRelativeToScrollingAncestor = flippedStickyBoxRect.location() + containingBlock->localToContainerQuad(FloatRect(FloatPoint(), containingBlock->size()), &enclosingClippingBox).boundingBox().location();
     507
     508    // Ideally, it would be possible to call this->localToContainerQuad to determine the frame
     509    // rectangle in the coordinate system of the scrolling ancestor, but localToContainerQuad
     510    // itself depends on sticky positioning! Instead, start from the parent but first adjusting
     511    // the rectangle for the writing mode of this stickily-positioned element.
     512    //
     513    // FIXME: For now, assume that |this| is not transformed. It would also be nice to not have to
     514    // call localToContainerQuad again since we have already done a similar call to move from
     515    // the containing block to the scrolling ancestor above, but localToContainerQuad takes care
     516    // of a lot of complex situations involving inlines, tables, and transformations.
     517    if (parent()->isBox())
     518        downcast<RenderBox>(parent())->flipForWritingMode(stickyBoxRect);
     519    auto stickyBoxRelativeToScrollingAncestor = parent()->localToContainerQuad(FloatRect(stickyBoxRect), &enclosingClippingBox).boundingBox();
     520
    508521    if (enclosingClippingLayer) {
    509         stickyLocationRelativeToScrollingAncestor -= FloatSize(enclosingClippingBox.borderLeft() + enclosingClippingBox.paddingLeft(),
    510             enclosingClippingBox.borderTop() + enclosingClippingBox.paddingTop());
    511         if (&enclosingClippingBox != containingBlock) {
     522        stickyBoxRelativeToScrollingAncestor.move(-FloatSize(enclosingClippingBox.borderLeft() + enclosingClippingBox.paddingLeft(),
     523            enclosingClippingBox.borderTop() + enclosingClippingBox.paddingTop()));
     524
     525        if (&enclosingClippingBox != parent()) {
    512526            if (auto* scrollableArea = enclosingClippingLayer->scrollableArea())
    513                 stickyLocationRelativeToScrollingAncestor += scrollableArea->scrollOffset();
    514         }
    515     }
    516     // FIXME: For now, assume that |this| is not transformed.
    517     stickyBoxRelativeToScrollingAnecstor.setLocation(stickyLocationRelativeToScrollingAncestor);
    518     constraints.setStickyBoxRect(stickyBoxRelativeToScrollingAnecstor);
     527                stickyBoxRelativeToScrollingAncestor.moveBy(scrollableArea->scrollOffset());
     528        }
     529    }
     530    constraints.setStickyBoxRect(stickyBoxRelativeToScrollingAncestor);
    519531
    520532    if (!style().left().isAuto()) {

  • trunk/Source/WebCore/rendering/RenderBoxModelObject.h

    r278443 r281185  
    117117
    118118    FloatRect constrainingRectForStickyPosition() const;
    119     const RenderBox& enclosingClippingBoxForStickyPosition(const RenderLayer** enclosingClippingLayer = nullptr) const;
     119    std::pair<const RenderBox&, const RenderLayer*> enclosingClippingBoxForStickyPosition() const;
    120120    void computeStickyPositionConstraints(StickyPositionViewportConstraints&, const FloatRect& constrainingRect) const;
    121121    LayoutSize stickyPositionOffset() const;
Note: See TracChangeset for help on using the changeset viewer.