| 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: | P2 | Keywords: | InRadar | ||||||||||
| Version: | Safari Technology Preview | ||||||||||||
| Hardware: | Unspecified | ||||||||||||
| OS: | Unspecified | ||||||||||||
| See Also: | https://github.com/web-platform-tests/wpt/pull/32796 | ||||||||||||
| Attachments: |
|
||||||||||||
The problem is that the children of the contain container are not being relayout after hover. Created attachment 451246 [details]
Patch
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 Created attachment 451279 [details]
Patch
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 Created attachment 451510 [details]
Patch
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]. 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. |
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.