Bug 236261

Summary:

Incorrect abspos layout when toggling contain

Product:

WebKit

Reporter:

Simon Fraser (smfr) <simon.fraser>

Component:

Layout and Rendering

Assignee:

Rob Buis <rbuis>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

bfulgham, cathiechen, changseok, clopez, esprehn+autocc, ews-watchlist, glenn, kondapallykalyan, pdr, rbuis, simon.fraser, webkit-bug-importer, youennf, zalan

Priority:

Keywords:

InRadar

Version:

Safari Technology Preview

Hardware:

Unspecified

OS:

Unspecified

See Also:

https://github.com/web-platform-tests/wpt/pull/32796

Attachments:

Description	Flags
testcase	none
Patch	none
Patch	none
Patch	none

Simon Fraser (smfr)

Reported 2022-02-07 14:37:28 PST

Created attachment 451153 [details] testcase Open the testcase and hover the black-outlined box. Note how the green box (abspos) moves to the wrong position.

Attachments
testcase (924 bytes, text/html) 2022-02-07 14:37 PST, Simon Fraser (smfr)	no flags	Details
Patch (5.31 KB, patch) 2022-02-08 07:50 PST, Rob Buis	no flags	Details Formatted Diff Diff
Patch (5.66 KB, patch) 2022-02-08 11:18 PST, Rob Buis	no flags	Details Formatted Diff Diff
Patch (5.76 KB, patch) 2022-02-10 03:19 PST, Rob Buis	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Rob Buis

Comment 1 2022-02-08 03:57:15 PST

The problem is that the children of the contain container are not being relayout after hover.

Rob Buis

Comment 2 2022-02-08 07:50:30 PST

Created attachment 451246 [details] Patch

EWS Watchlist

Comment 3 2022-02-08 07:52:34 PST

This patch modifies the imported WPT tests. Please ensure that any changes on the tests (not coming from a WPT import) are exported to WPT. Please see https://trac.webkit.org/wiki/WPTExportProcess

Rob Buis

Comment 4 2022-02-08 11:18:15 PST

Created attachment 451279 [details] Patch

Simon Fraser (smfr)

Comment 5 2022-02-09 12:17:19 PST

Comment on attachment 451279 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=451279&action=review > Source/WebCore/rendering/RenderBlock.cpp:381 > + if (oldStyle.position() == newStyle.position() && hadTransform == willHaveTransform && (hadLayoutContainment == willHaveLayoutContainment)) No need for the last pair of parens. > Source/WebCore/rendering/RenderBlock.cpp:387 > + outOfFlowDescendantsHaveNewContainingBlock = hadLayoutContainment && !willHaveLayoutContainment; I can't figure out why it's hadLayoutContainment && !willHaveLayoutContainment and not also !hadLayoutContainment && willHaveLayoutContainment > LayoutTests/imported/w3c/web-platform-tests/css/css-contain/contain-layout-020.html:38 > + window.requestAnimationFrame( () => { No space before () unless this is WPT style. I'd drop 'window.' too

Rob Buis

Comment 6 2022-02-10 03:19:56 PST

Created attachment 451510 [details] Patch

EWS

Comment 7 2022-02-10 05:03:18 PST

Committed r289527 (247058@main): <https://commits.webkit.org/247058@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 451510 [details].

Radar WebKit Bug Importer

Comment 8 2022-02-10 05:04:17 PST

<rdar://problem/88749136>

Rob Buis

Comment 9 2022-02-10 05:07:22 PST

Comment on attachment 451279 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=451279&action=review >> Source/WebCore/rendering/RenderBlock.cpp:381 >> + if (oldStyle.position() == newStyle.position() && hadTransform == willHaveTransform && (hadLayoutContainment == willHaveLayoutContainment)) > > No need for the last pair of parens. Fixed. >> Source/WebCore/rendering/RenderBlock.cpp:387 >> + outOfFlowDescendantsHaveNewContainingBlock = hadLayoutContainment && !willHaveLayoutContainment; > > I can't figure out why it's hadLayoutContainment && !willHaveLayoutContainment and not also !hadLayoutContainment && willHaveLayoutContainment Yes, this was tricky. The idea is in the case of !hadLayoutContainment && willHaveLayoutContainment we want to enter the below block "We are a new containing block.". >> LayoutTests/imported/w3c/web-platform-tests/css/css-contain/contain-layout-020.html:38 >> + window.requestAnimationFrame( () => { > > No space before () unless this is WPT style. I'd drop 'window.' too Fixed.

Note You need to log in before you can comment on or make changes to this bug.