Bug 249391 - Fix inline-block abspos bug
Summary: Fix inline-block abspos bug
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Layout and Rendering (show other bugs)
Version: Safari Technology Preview
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords: BrowserCompat, InRadar
Depends on:
Blocks:
 
Reported: 2022-12-15 07:08 PST by Ahmad Saleem
Modified: 2023-03-23 16:08 PDT (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Ahmad Saleem 2022-12-15 07:08:32 PST
Hi Team,

While going through Blink commit and came across another failing test case:

Test Case - https://jsfiddle.net/zbusvq2g/ (Broken in Safari 16.2 & STP160)

^ Working fine in Chrome Canary 110 and Firefox Nightly 110.

Blink Commit - https://chromium.googlesource.com/chromium/blink/+/b1dbd0d86f09b9dccd410d1d106c76f029fa5b48

Webkit GitHub Source - https://github.com/WebKit/WebKit/blob/d80ae68ecc7a4a5756a38de4a2eb3d2360375012/Source/WebCore/rendering/RenderLayerModelObject.cpp#L130

Just wanted to raise this bug for future merge or tracking.

Thanks!
Comment 1 Radar WebKit Bug Importer 2022-12-22 07:09:17 PST
<rdar://problem/103637239>
Comment 2 Ahmad Saleem 2023-02-24 18:02:42 PST
This seems to be repaint issue because resizing window make it looks similar to Chrome Canary 112 and Firefox Nightly 112.

I tried in local testing and this compiles:

    if ((oldStyle && isOutOfFlowPositioned() && parent() && (parent() != containingBlock()))
        && (style().position() == oldStyle->position())
        && (style().display() != oldStyle->display())
        && ((style().display() == DisplayType::Block) || (style().display() == DisplayType::InlineBlock))
        && ((oldStyle->display() == DisplayType::Block ) || (oldStyle->display() == DisplayType::InlineBlock)))
            parent()->setChildNeedsLayout(MarkOnlyThis);

_______

I tried to change "last line" with:

setNeedsLayout() and other child variants to force repainting to get this fixed but it seems it does not work in case of Safari.

Would appreciate any input or suggestion. Just wanted to share my input and progress in the background on fixing this bug. Thanks!
Comment 3 zalan 2023-02-25 08:20:40 PST
(In reply to Ahmad Saleem from comment #2)
> This seems to be repaint issue because resizing window make it looks similar
> to Chrome Canary 112 and Firefox Nightly 112.
> 
> I tried in local testing and this compiles:
> 
>     if ((oldStyle && isOutOfFlowPositioned() && parent() && (parent() !=
> containingBlock()))
>         && (style().position() == oldStyle->position())
>         && (style().display() != oldStyle->display())
>         && ((style().display() == DisplayType::Block) || (style().display()
> == DisplayType::InlineBlock))
>         && ((oldStyle->display() == DisplayType::Block ) ||
> (oldStyle->display() == DisplayType::InlineBlock)))
>             parent()->setChildNeedsLayout(MarkOnlyThis);
> 
> _______
> 
> I tried to change "last line" with:
> 
> setNeedsLayout() and other child variants to force repainting to get this
> fixed but it seems it does not work in case of Safari.
> 
> Would appreciate any input or suggestion. Just wanted to share my input and
> progress in the background on fixing this bug. Thanks!
I think parent()->setNeedsLayout(LayoutInvalidationReason::ChildChanged, MarkContainerChain); maps to parent()->setChildNeedsLayout();
Comment 4 Ahmad Saleem 2023-02-25 09:17:15 PST
(In reply to zalan from comment #3)
> (In reply to Ahmad Saleem from comment #2)
> > This seems to be repaint issue because resizing window make it looks similar
> > to Chrome Canary 112 and Firefox Nightly 112.
> > 
> > I tried in local testing and this compiles:
> > 
> >     if ((oldStyle && isOutOfFlowPositioned() && parent() && (parent() !=
> > containingBlock()))
> >         && (style().position() == oldStyle->position())
> >         && (style().display() != oldStyle->display())
> >         && ((style().display() == DisplayType::Block) || (style().display()
> > == DisplayType::InlineBlock))
> >         && ((oldStyle->display() == DisplayType::Block ) ||
> > (oldStyle->display() == DisplayType::InlineBlock)))
> >             parent()->setChildNeedsLayout(MarkOnlyThis);
> > 
> > _______
> > 
> > I tried to change "last line" with:
> > 
> > setNeedsLayout() and other child variants to force repainting to get this
> > fixed but it seems it does not work in case of Safari.
> > 
> > Would appreciate any input or suggestion. Just wanted to share my input and
> > progress in the background on fixing this bug. Thanks!
> I think parent()->setNeedsLayout(LayoutInvalidationReason::ChildChanged,
> MarkContainerChain); maps to parent()->setChildNeedsLayout();

Just tried on local build. Still it does have repaint bug where resizing make it aligned with Chrome Canary 112 and Firefox Nightly 112.
Comment 5 Ahmad Saleem 2023-03-21 16:09:43 PDT
(In reply to Ahmad Saleem from comment #4)
> (In reply to zalan from comment #3)
> > (In reply to Ahmad Saleem from comment #2)
> > > This seems to be repaint issue because resizing window make it looks similar
> > > to Chrome Canary 112 and Firefox Nightly 112.
> > > 
> > > I tried in local testing and this compiles:
> > > 
> > >     if ((oldStyle && isOutOfFlowPositioned() && parent() && (parent() !=
> > > containingBlock()))
> > >         && (style().position() == oldStyle->position())
> > >         && (style().display() != oldStyle->display())
> > >         && ((style().display() == DisplayType::Block) || (style().display()
> > > == DisplayType::InlineBlock))
> > >         && ((oldStyle->display() == DisplayType::Block ) ||
> > > (oldStyle->display() == DisplayType::InlineBlock)))
> > >             parent()->setChildNeedsLayout(MarkOnlyThis);
> > > 
> > > _______
> > > 
> > > I tried to change "last line" with:
> > > 
> > > setNeedsLayout() and other child variants to force repainting to get this
> > > fixed but it seems it does not work in case of Safari.
> > > 
> > > Would appreciate any input or suggestion. Just wanted to share my input and
> > > progress in the background on fixing this bug. Thanks!
> > I think parent()->setNeedsLayout(LayoutInvalidationReason::ChildChanged,
> > MarkContainerChain); maps to parent()->setChildNeedsLayout();
> 
> Just tried on local build. Still it does have repaint bug where resizing
> make it aligned with Chrome Canary 112 and Firefox Nightly 112.

FIXED it:

if ((oldStyle && isOutOfFlowPositioned() && parent() && (parent() != containingBlock()))
        && (style().position() == oldStyle->position())
        && (style().isOriginalDisplayInlineType() != oldStyle->isOriginalDisplayInlineType())
        && ((style().isOriginalDisplayBlockType()) || (style().isOriginalDisplayInlineType()))
        && ((oldStyle->isOriginalDisplayBlockType()) || (oldStyle->isOriginalDisplayInlineType())))
            parent()->setChildNeedsLayout();
Comment 6 Ahmad Saleem 2023-03-21 16:41:35 PDT
PR - https://github.com/WebKit/WebKit/pull/11781/files

In local testing, it passes this feeling test from JSFiddle of Comment 0. Running through EWS and will rebase any failing test tomorrow morning.
Comment 7 EWS 2023-03-23 16:08:00 PDT
Committed 262042@main (47b7f1121f2e): <https://commits.webkit.org/262042@main>

Reviewed commits have been landed. Closing PR #11781 and removing active labels.