Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[IFC] List bullets are aligned differently within a list when the lis…
…t item has a flex container

https://bugs.webkit.org/show_bug.cgi?id=258414
<rdar://111066602>

Reviewed by Antti Koivisto.

Let's account for the box logical left position when computing the adjustment for outside list markers inside flex/grid.

In many cases outside list marker renderers end up inside descendant blocks ("closest" to the inline content on line) while their
horizontal position should ignore such "embedding".
Essentially they are supposed to show up at the same horizontal position regardless of how many generated/non-generated blocks
there are between the (tree) position of the RenderListMarker and the associated RenderListItem.
e.g.
  <li>
    <div style="display: flex;">
      <div style="width: 100px; height: 100px;"></div>
      <div id=inline_content>this is where the list marker gets inserted</div>
    </div>
  </li>
While the list marker is visually placed at the left side of <li>, it is placed under <div id=inline_content>.

offsetFromParentListItem is supposed to resolve this by climbing up the ancestor chain and accumulating
padding and border on the (logical) left between the RenderListMarker and RenderListItem. With flex/grid layout there
could be some additional offset between those renderers (other flex/grid items).
This patch fixes such cases by taking logical left into account for such content (sadly we can't have a uniform approach to this since
(in the render tree) block content does not get its final position until after this code runs. Moreover stale logicalLeft (leftover from previous
layout could lead to misplaced list markers. Legacy line layout addresses this by computing this offset (and moving the list marker!) _after_
layout at overflow computation time (see RenderListMarker::addOverflowFromListMarker)
In full LFC this will be fixed by inserting the marker box where it belongs).

* LayoutTests/fast/inline/list-marker-inside-flex-expected.html: Added.
* LayoutTests/fast/inline/list-marker-inside-flex.html: Added.
* Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp:
(WebCore::LayoutIntegration::LineLayout::updateListMarkerDimensions):

Canonical link: https://commits.webkit.org/265464@main
  • Loading branch information
alanbaradlay committed Jun 23, 2023
1 parent 4b893ed commit 1c78a5c
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 1 deletion.
16 changes: 16 additions & 0 deletions LayoutTests/fast/inline/list-marker-inside-flex-expected.html
@@ -0,0 +1,16 @@
<style>
div {
color: transparent;
}
</style>
<ul>
<li>
<div style="display: inline-block; background-color: green; width: 100px; height: 100px;">PASS</div>
</li>
<li style="padding-left: 10px;">
<div style="display: inline-block; background-color: green; width: 100px; height: 100px;">PASS</div>
</li>
<li style="padding-left: 10px;">
<div style="display: inline-block; background-color: green; width: 100px; height: 100px;">PASS</div>
</li>
</ul>
25 changes: 25 additions & 0 deletions LayoutTests/fast/inline/list-marker-inside-flex.html
@@ -0,0 +1,25 @@
<style>
div {
color: transparent;
}
</style>
<ul>
<li>
<div style="display: flex;">
<div style="background-color: green; width: 100px; height: 100px;"></div>
<div>PASS if marker is outside of the green box</div>
</div>
</li>
<li>
<div style="display: flex; padding-left: 10px;">
<div style="background-color: green; width: 100px; height: 100px;"></div>
<div>PASS if marker is outside of the green box</div>
</div>
</li>
<li>
<div style="display: flex; padding-left: 10px;">
<div style="background-color: green; width: 100px; height: 100px;"></div>
<div style="padding-left: 40px;">PASS if marker is outside of the green box</div>
</div>
</li>
</ul>
Expand Up @@ -243,11 +243,19 @@ void LineLayout::updateListMarkerDimensions(const RenderListMarker& listMarker)
if (layoutBox.isListMarkerOutside()) {
auto* ancestor = listMarker.containingBlock();
auto offsetFromParentListItem = [&] {
auto hasAccountedForBorderAndPadding = false;
auto offset = LayoutUnit { };
for (; ancestor; ancestor = ancestor->containingBlock()) {
offset -= (ancestor->borderStart() + ancestor->paddingStart());
if (!hasAccountedForBorderAndPadding)
offset -= (ancestor->borderStart() + ancestor->paddingStart());
if (is<RenderListItem>(*ancestor))
break;
if (ancestor->isFlexItem()) {
offset -= ancestor->logicalLeft();
hasAccountedForBorderAndPadding = true;
continue;
}
hasAccountedForBorderAndPadding = false;
}
return offset;
}();
Expand Down

0 comments on commit 1c78a5c

Please sign in to comment.