[List Marker] Collapse anonymous blocks for outside marker when blockification prevents line-box parenting#60215
Conversation
|
EWS run on previous version of this PR (hash 73b7091) Details
|
LayoutTests/fast/lists/list-marker-outside-flex-collapse-anonymous-block.html
Outdated
Show resolved
Hide resolved
73b7091 to
a0907e7
Compare
|
EWS run on previous version of this PR (hash a0907e7) Details
|
macOS Safer C++ Build #85184 (a0907e7)❌ Found 1 failing file with 2 issues. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming. |
iOS Safer C++ Build #3416 (a0907e7)❌ Found 1 failing file with 2 issues. Please address these issues before landing. See WebKit Guidelines for Safer C++ Programming. |
| if (rootBox.isAnonymous() && lineContent[0].isListMarkerOutside()) { | ||
| // When an outside marker ends up in an anonymous block because blockification (e.g., by a flex/grid container) | ||
| // prevented finding a line box parent, collapse the line box so it doesn't inflate the list item. | ||
| auto* markerRenderer = dynamicDowncast<RenderListMarker>(lineContent[0].layoutBox().rendererForIntegration()); |
There was a problem hiding this comment.
We try not to access renderers from inside IFC -well except the integration layer- but transfer such states from renderers to layout boxes while building the tree. ElementBox already has examples like this see setListMarkerLayoutBounds. I would also fold this logic under if (lineContent[0].isListMarkerOutside()... -few lines above.
There was a problem hiding this comment.
Done, thanks for the review!
a0907e7 to
e352452
Compare
|
EWS run on previous version of this PR (hash e352452) Details
|
Source/WebCore/layout/formattingContexts/inline/InlineQuirks.cpp
Outdated
Show resolved
Hide resolved
Source/WebCore/layout/formattingContexts/inline/InlineQuirks.cpp
Outdated
Show resolved
Hide resolved
e352452 to
b3d1801
Compare
|
EWS run on previous version of this PR (hash b3d1801) Details
|
b3d1801 to
03e0dd4
Compare
|
EWS run on previous version of this PR (hash 03e0dd4) Details |
03e0dd4 to
bb98bd6
Compare
|
EWS run on previous version of this PR (hash bb98bd6) Details
|
bb98bd6 to
43b891e
Compare
|
EWS run on previous version of this PR (hash 43b891e) Details
|
43b891e to
f0bebfa
Compare
|
EWS run on previous version of this PR (hash f0bebfa) Details |
| if (!lineContent[0].isListMarkerOutside()) { | ||
| ASSERT(lineContent[0].isListMarkerInside()); | ||
| auto& marker = lineContent[0]; | ||
| auto* markerBox = dynamicDowncast<Layout::ElementBox>(lineContent[0].layoutBox()); |
f0bebfa to
53a8ef8
Compare
|
EWS run on current version of this PR (hash 53a8ef8) Details |
|
Safe-Merge-Queue: Build #86611. |
…ification prevents line-box parenting https://bugs.webkit.org/show_bug.cgi?id=309925 <rdar://165933903> Reviewed by Alan Baradlay. findParentOfEmptyOrFirstLineBox sometimes fails to find a suitable line-box parent when a list item contains a flex or grid container because flex/grid containers blockify inline content like <svg> and <img>. In this case, we fall back to creating an anonymous block to parent the list marker. However, this causes layout issues because the anonymous block's inherited line height pushes the visible content down (and no longer next to the list marker). This fixes the issue by detecting parenting failure due to blockification and flagging the marker to collapse its parent block. * LayoutTests/fast/lists/list-marker-outside-flex-collapse-anonymous-block-expected.html: Added. * LayoutTests/fast/lists/list-marker-outside-flex-collapse-anonymous-block.html: Added. * Source/WebCore/layout/formattingContexts/inline/InlineQuirks.cpp: (WebCore::Layout::InlineQuirks::shouldCollapseLineBoxHeight const): * Source/WebCore/layout/integration/LayoutIntegrationBoxTreeUpdater.cpp: (WebCore::LayoutIntegration::updateListMarkerAttributes): (WebCore::LayoutIntegration::BoxTreeUpdater::createLayoutBox): * Source/WebCore/layout/layouttree/LayoutElementBox.h: (WebCore::Layout::ElementBox::shouldCollapseAnonymousBlockParentForListMarker const): * Source/WebCore/rendering/RenderListMarker.h: * Source/WebCore/rendering/updating/RenderTreeBuilderList.cpp: (WebCore::findParentOfEmptyOrFirstLineBox): (WebCore::parentCandidateForMarker): (WebCore::RenderTreeBuilder::List::updateItemMarker): Canonical link: https://commits.webkit.org/309272@main
53a8ef8 to
376d3ef
Compare
|
Committed 309272@main (376d3ef): https://commits.webkit.org/309272@main Reviewed commits have been landed. Closing PR #60215 and removing active labels. |
🛠 mac-apple
376d3ef
53a8ef8
🧪 win-tests