Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
[IFC][list marker] Adjust nested list markers when intrusive float is…
… present

https://bugs.webkit.org/show_bug.cgi?id=249975

Reviewed by Antti Koivisto.

The following html markup

  <!DOCTYPE html>
  <ul><li><ul><li>overlapping nested list markers</li></ul></li></ul>

produces 2 list markers. These markers are placed on the same line.

   *  *  overlapping nested list markers.

However in quirks mode:

  <ul><li><ul><li>non-overlapping nested list markers</li></ul></li></ul>

each list maker is on a dedicated line.

   *
      *  non-overlapping nested list markers

This is controlled by negative margin start on the list marker box.

These list marker positions are affected by any overlapping floats, even when the float
is not intrusive to the line itself.

This patch makes sure the we take floats into account when computing the final position of such list items.

LineLayout::updateListMarkerDimensions: Cache the list marker's offset from the associated list item.
In case of nested list markers (when multiple markers share the same line), the containing block is not
necessarily the associated <li>. This offset is later may be adjusted by taking overlapping floats into account.

LineBoxBuilder::adjustOutsideListMarkersPosition: In this function we compute the final position of the list markers
and also make that nested list markers do avoid float boxes.

* LayoutTests/fast/lists/overlapping-nested-list-markers-expected.html: Added.
* LayoutTests/fast/lists/overlapping-nested-list-markers.html: Added.
* Source/WebCore/layout/formattingContexts/block/BlockLayoutState.h:
(WebCore::Layout::BlockLayoutState::floatingState const):
* Source/WebCore/layout/formattingContexts/inline/InlineFormattingContext.cpp:
(WebCore::Layout::InlineFormattingContext::createDisplayContentForLine):
* Source/WebCore/layout/formattingContexts/inline/InlineFormattingGeometry.cpp:
(WebCore::Layout::InlineFormattingGeometry::adjustMarginStartForListMarker const):
* Source/WebCore/layout/formattingContexts/inline/InlineFormattingGeometry.h:
* Source/WebCore/layout/formattingContexts/inline/InlineFormattingState.h:
(WebCore::Layout::InlineFormattingState::addNestedListMarkerOffset):
(WebCore::Layout::InlineFormattingState::nestedListMarkerOffset const):
(WebCore::Layout::InlineFormattingState::resetNestedListMarkerOffsets):
* Source/WebCore/layout/formattingContexts/inline/InlineLevelBox.h:
(WebCore::Layout::InlineLevelBox::setLogicalLeft):
* Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.cpp:
(WebCore::Layout::LineBoxBuilder::LineBoxBuilder):
(WebCore::Layout::LineBoxBuilder::build):
(WebCore::Layout::LineBoxBuilder::constructInlineLevelBoxes):
(WebCore::Layout::LineBoxBuilder::computeLineBoxGeometry const):
(WebCore::Layout::LineBoxBuilder::adjustOutsideListMarkersPosition):
* Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.h:
(WebCore::Layout::LineBoxBuilder::blockLayoutState const):
* Source/WebCore/layout/integration/LayoutIntegrationCoverage.cpp:
(WebCore::LayoutIntegration::canUseForChild):
* Source/WebCore/layout/integration/inline/LayoutIntegrationLineLayout.cpp:
(WebCore::LayoutIntegration::LineLayout::updateListMarkerDimensions):
(WebCore::LayoutIntegration::LineLayout::constructContent):

Canonical link: https://commits.webkit.org/258372@main
  • Loading branch information
alanbaradlay committed Jan 1, 2023
1 parent 0611819 commit 992e7fe
Show file tree
Hide file tree
Showing 18 changed files with 159 additions and 51 deletions.
@@ -0,0 +1,14 @@
<!DOCTYPE html>
<style>
ul {
list-style-type: circle;
}
.container {
padding-left: 160px;
}
</style>
<div class=container>
<ul><li>overlapping nested list markers</li></ul>
<ul><li>overlapping nested list markers</li></ul>
<ul><li>overlapping nested list markers</li></ul>
</div>
16 changes: 16 additions & 0 deletions LayoutTests/fast/lists/overlapping-nested-list-markers.html
@@ -0,0 +1,16 @@
<!DOCTYPE html>
<meta name="fuzzy" content="maxDifference=120-121; totalPixels=54-117" />
<style>
ul {
list-style-type: circle;
}
.float {
float: left;
width: 200px;
height: 500px;
}
</style>
<div class=float></div>
<ul><li><ul><li>overlapping nested list markers</li></ul></li></ul>
<ul><li><ul><li><ul><li>overlapping nested list markers</li></ul></li></ul></li></ul>
<ul><li><ul><li><ul><li><ul><li>overlapping nested list markers</li></ul></li></ul></li></ul></li></ul>
Expand Up @@ -53,12 +53,12 @@ layer at (0,0) size 800x1233
text run at (80,40) width 508: "The text has been right-aligned in order to make the right padding easier to see."
RenderBlock {UL} at (0,491) size 784x80 [bgcolor=#808080]
RenderListItem {LI} at (40,0) size 719x40
RenderListMarker at (-18,0) size 8x19: bullet
RenderListMarker at (-18,0) size 7x19: bullet
RenderText {#text} at (7,0) size 712x39
text run at (7,0) width 712: "The right padding on this unordered list has been set to 25 pixels, which will require some extra text in order to"
text run at (692,20) width 27: "test."
RenderListItem {LI} at (40,40) size 719x40 [bgcolor=#FFFFFF]
RenderListMarker at (-18,0) size 8x19: bullet
RenderListMarker at (-18,0) size 7x19: bullet
RenderText {#text} at (21,0) size 673x39
text run at (21,0) width 673: "This list item has a right padding of 25 pixels, which will appear to the left of the gray padding of the UL"
text run at (639,20) width 55: "element."
Expand Down Expand Up @@ -120,7 +120,7 @@ layer at (0,0) size 800x1233
text run at (1,0) width 696: "The right padding on this unordered list has been set to 25 pixels, which will require some extra text in order"
text run at (654,20) width 43: "to test."
RenderListItem {LI} at (40,40) size 697x40 [bgcolor=#FFFFFF]
RenderListMarker at (-18,0) size 8x19: bullet
RenderListMarker at (-18,0) size 7x19: bullet
RenderText {#text} at (24,0) size 648x39
text run at (24,0) width 648: "This list item has a right padding of 25 pixels, which will appear to the left of the gray padding of the"
text run at (592,20) width 80: "UL element."
Expand Down
4 changes: 2 additions & 2 deletions LayoutTests/platform/ios/fast/lists/002-expected.txt
Expand Up @@ -6,12 +6,12 @@ layer at (0,0) size 800x600
RenderBlock {DIV} at (0,0) size 784x40
RenderBlock {UL} at (0,0) size 784x40
RenderListItem {LI} at (40,0) size 744x20
RenderListMarker at (-18,0) size 8x19: bullet
RenderListMarker at (-18,0) size 7x19: bullet
RenderInline {A} at (0,0) size 40x19 [color=#0000EE]
RenderText {#text} at (704,0) size 40x19
text run at (704,0) width 40: "Home"
RenderListItem {LI} at (40,20) size 744x20
RenderListMarker at (-18,0) size 8x19: bullet
RenderListMarker at (-18,0) size 7x19: bullet
RenderInline {A} at (0,0) size 58x19 [color=#0000EE]
RenderText {#text} at (686,0) size 58x19
text run at (686,0) width 58: "Archives"
Expand Down
4 changes: 2 additions & 2 deletions LayoutTests/platform/ios/fast/lists/002-vertical-expected.txt
Expand Up @@ -6,12 +6,12 @@ layer at (0,0) size 800x600
RenderBlock {DIV} at (0,0) size 40x584
RenderBlock {UL} at (0,0) size 40x584
RenderListItem {LI} at (0,40) size 20x544
RenderListMarker at (0,-18) size 19x8: bullet
RenderListMarker at (0,-18) size 19x7: bullet
RenderInline {A} at (0,0) size 19x40 [color=#0000EE]
RenderText {#text} at (0,504) size 19x40
text run at (0,504) width 40: "Home"
RenderListItem {LI} at (20,40) size 20x544
RenderListMarker at (0,-18) size 19x8: bullet
RenderListMarker at (0,-18) size 19x7: bullet
RenderInline {A} at (0,0) size 19x58 [color=#0000EE]
RenderText {#text} at (0,486) size 19x58
text run at (0,486) width 58: "Archives"
Expand Down
Expand Up @@ -38,12 +38,12 @@ layer at (0,0) size 785x1023
text run at (396,18) width 181: "width of the parent element."
RenderBlock {UL} at (0,343) size 744x108 [bgcolor=#808080]
RenderListItem {LI} at (40,0) size 704x36
RenderListMarker at (-17,0) size 8x18: bullet
RenderListMarker at (-17,0) size 7x18: bullet
RenderText {#text} at (27,0) size 677x36
text run at (27,0) width 677: "The right margin on this unordered list has been set to 25 pixels, and the background color has been set to"
text run at (672,18) width 32: "gray."
RenderListItem {LI} at (40,36) size 679x54 [bgcolor=#FFFFFF]
RenderListMarker at (-17,0) size 8x18: bullet
RenderListMarker at (-17,0) size 7x18: bullet
RenderText {#text} at (10,0) size 669x54
text run at (10,0) width 669: "Another list item might not be such a bad idea, either, considering that such things do need to be double-"
text run at (34,18) width 62: "checked. "
Expand Down Expand Up @@ -93,12 +93,12 @@ layer at (0,0) size 785x1023
text run at (356,18) width 205: "the width of the parent element."
RenderBlock {UL} at (4,192) size 722x108 [bgcolor=#808080]
RenderListItem {LI} at (40,0) size 682x36
RenderListMarker at (-17,0) size 8x18: bullet
RenderListMarker at (-17,0) size 7x18: bullet
RenderText {#text} at (5,0) size 677x36
text run at (5,0) width 677: "The right margin on this unordered list has been set to 25 pixels, and the background color has been set to"
text run at (650,18) width 32: "gray."
RenderListItem {LI} at (40,36) size 657x54 [bgcolor=#FFFFFF]
RenderListMarker at (-17,0) size 8x18: bullet
RenderListMarker at (-17,0) size 7x18: bullet
RenderText {#text} at (40,0) size 637x54
text run at (40,0) width 617: "Another list item might not be such a bad idea, either, considering that such things do need to be"
text run at (20,18) width 110: "double-checked. "
Expand Down
Expand Up @@ -116,12 +116,12 @@ layer at (0,0) size 785x1171
text run at (478,54) width 83: "easier to see."
RenderBlock {UL} at (4,336) size 747x72 [bgcolor=#808080]
RenderListItem {LI} at (40,0) size 682x36
RenderListMarker at (-17,0) size 8x18: bullet
RenderListMarker at (-17,0) size 7x18: bullet
RenderText {#text} at (24,0) size 658x36
text run at (24,0) width 658: "The right padding on this unordered list has been set to 25 pixels, which will require some extra text in"
text run at (601,18) width 81: "order to test."
RenderListItem {LI} at (40,36) size 682x36 [bgcolor=#FFFFFF]
RenderListMarker at (-17,0) size 8x18: bullet
RenderListMarker at (-17,0) size 7x18: bullet
RenderText {#text} at (9,0) size 648x36
text run at (9,0) width 648: "This list item has a right padding of 25 pixels, which will appear to the left of the gray padding of the"
text run at (577,18) width 80: "UL element."
Expand Down
4 changes: 2 additions & 2 deletions LayoutTests/platform/mac/fast/lists/002-expected.txt
Expand Up @@ -6,12 +6,12 @@ layer at (0,0) size 800x600
RenderBlock {DIV} at (0,0) size 784x36
RenderBlock {UL} at (0,0) size 784x36
RenderListItem {LI} at (40,0) size 744x18
RenderListMarker at (-17,0) size 8x18: bullet
RenderListMarker at (-17,0) size 7x18: bullet
RenderInline {A} at (0,0) size 40x18 [color=#0000EE]
RenderText {#text} at (704,0) size 40x18
text run at (704,0) width 40: "Home"
RenderListItem {LI} at (40,18) size 744x18
RenderListMarker at (-17,0) size 8x18: bullet
RenderListMarker at (-17,0) size 7x18: bullet
RenderInline {A} at (0,0) size 58x18 [color=#0000EE]
RenderText {#text} at (686,0) size 58x18
text run at (686,0) width 58: "Archives"
Expand Down
4 changes: 2 additions & 2 deletions LayoutTests/platform/mac/fast/lists/002-vertical-expected.txt
Expand Up @@ -6,12 +6,12 @@ layer at (0,0) size 800x600
RenderBlock {DIV} at (0,0) size 36x584
RenderBlock {UL} at (0,0) size 36x584
RenderListItem {LI} at (0,40) size 18x544
RenderListMarker at (0,-17) size 18x8: bullet
RenderListMarker at (0,-17) size 18x7: bullet
RenderInline {A} at (0,0) size 18x40 [color=#0000EE]
RenderText {#text} at (0,504) size 18x40
text run at (0,504) width 40: "Home"
RenderListItem {LI} at (18,40) size 18x544
RenderListMarker at (0,-17) size 18x8: bullet
RenderListMarker at (0,-17) size 18x7: bullet
RenderInline {A} at (0,0) size 18x58 [color=#0000EE]
RenderText {#text} at (0,486) size 18x58
text run at (0,486) width 58: "Archives"
Expand Down
Expand Up @@ -48,6 +48,8 @@ class BlockLayoutState {
BlockLayoutState(FloatingState&, std::optional<LineClamp> = { }, LeadingTrim = { });

FloatingState& floatingState() { return m_floatingState; }
const FloatingState& floatingState() const { return m_floatingState; }

std::optional<LineClamp> lineClamp() const { return m_lineClamp; }
LeadingTrim leadingTrim() const { return m_leadingTrim; }

Expand Down
Expand Up @@ -473,7 +473,7 @@ InlineRect InlineFormattingContext::createDisplayContentForLine(const LineBuilde
auto& formattingState = this->formattingState();
auto currentLineIndex = formattingState.lines().size();

auto lineBox = LineBoxBuilder { *this, lineContent, blockLayoutState.leadingTrim() }.build(currentLineIndex);
auto lineBox = LineBoxBuilder { *this, lineContent, blockLayoutState }.build(currentLineIndex);
auto displayLine = InlineDisplayLineBuilder { *this }.build(lineContent, lineBox, constraints);
formattingState.addBoxes(InlineDisplayContentBuilder { *this, formattingState }.build(lineContent, lineBox, displayLine, currentLineIndex));
formattingState.addLineBox(WTFMove(lineBox));
Expand Down
Expand Up @@ -343,14 +343,14 @@ FloatingContext::Constraints InlineFormattingGeometry::floatConstraintsForLine(I
return floatingContext.constraints(toLayoutUnit(lineLogicalTop), toLayoutUnit(lineLogicalTop + contentLogicalHeight), FloatingContext::MayBeAboveLastFloat::Yes);
}

void InlineFormattingGeometry::adjustMarginStartForListMarker(const ElementBox& listMarkerBox, LayoutUnit rootInlineBoxOffset) const
void InlineFormattingGeometry::adjustMarginStartForListMarker(const ElementBox& listMarkerBox, LayoutUnit nestedListMarkerMarginStart, InlineLayoutUnit rootInlineBoxOffset) const
{
if (!rootInlineBoxOffset)
if (!nestedListMarkerMarginStart && !rootInlineBoxOffset)
return;
auto& listMarkerGeometry = const_cast<InlineFormattingState&>(formattingContext().formattingState()).boxGeometry(listMarkerBox);
// Make sure that the line content does not get pulled in to logical left direction due to
// the large negative margin (i.e. this ensures that logical left of the list content stays at the line start)
listMarkerGeometry.setHorizontalMargin({ listMarkerGeometry.marginStart() - rootInlineBoxOffset, listMarkerGeometry.marginEnd() + rootInlineBoxOffset });
listMarkerGeometry.setHorizontalMargin({ listMarkerGeometry.marginStart() + nestedListMarkerMarginStart - LayoutUnit { rootInlineBoxOffset }, listMarkerGeometry.marginEnd() - nestedListMarkerMarginStart + LayoutUnit { rootInlineBoxOffset } });
}

}
Expand Down
Expand Up @@ -58,7 +58,7 @@ class InlineFormattingGeometry : public FormattingGeometry {
LayoutPoint staticPositionForOutOfFlowInlineLevelBox(const Box&, LayoutPoint contentBoxTopLeft) const;
LayoutPoint staticPositionForOutOfFlowBlockLevelBox(const Box&, LayoutPoint contentBoxTopLeft) const;

void adjustMarginStartForListMarker(const ElementBox&, LayoutUnit rootInlineBoxOffset) const;
void adjustMarginStartForListMarker(const ElementBox&, LayoutUnit nestedListMarkerMarginStart, InlineLayoutUnit rootInlineBoxOffset) const;

private:
const InlineFormattingContext& formattingContext() const { return downcast<InlineFormattingContext>(FormattingGeometry::formattingContext()); }
Expand Down
Expand Up @@ -69,12 +69,18 @@ class InlineFormattingState : public FormattingState {
void clearLineAndBoxes();
void shrinkToFit();

void addNestedListMarkerOffset(const ElementBox& listMarkerBox, LayoutUnit offset) { m_nestedListMarkerOffset.add(&listMarkerBox, offset); }
LayoutUnit nestedListMarkerOffset(const ElementBox& listMarkerBox) const { return m_nestedListMarkerOffset.get(&listMarkerBox); }
void resetNestedListMarkerOffsets() { return m_nestedListMarkerOffset.clear(); }

private:
// Cacheable input to line layout.
InlineItems m_inlineItems;
InlineLineBoxes m_lineBoxes;
DisplayLines m_displayLines;
DisplayBoxes m_displayBoxes;
// FIXME: This should be part of a non-persistent formatting state.
HashMap<const ElementBox*, LayoutUnit> m_nestedListMarkerOffset;
InlineLayoutUnit m_clearGapAfterLastLine { 0 };
};

Expand Down
Expand Up @@ -132,6 +132,7 @@ class InlineLevelBox {
void setLogicalWidth(InlineLayoutUnit logicalWidth) { m_logicalRect.setWidth(logicalWidth); }
void setLogicalHeight(InlineLayoutUnit logicalHeight) { m_logicalRect.setHeight(roundToInt(logicalHeight)); }
void setLogicalTop(InlineLayoutUnit logicalTop) { m_logicalRect.setTop(logicalTop >= 0 ? roundToInt(logicalTop) : -roundToInt(-logicalTop)); }
void setLogicalLeft(InlineLayoutUnit logicalLeft) { m_logicalRect.setLeft(logicalLeft); }
void setAscent(InlineLayoutUnit ascent) { m_ascent = roundToInt(ascent); }
void setDescent(InlineLayoutUnit descent) { m_descent = roundToInt(descent); }
void setLayoutBounds(const LayoutBounds& layoutBounds) { m_layoutBounds = { InlineLayoutUnit(roundToInt(layoutBounds.ascent)), InlineLayoutUnit(roundToInt(layoutBounds.descent)) }; }
Expand Down

0 comments on commit 992e7fe

Please sign in to comment.