Skip to content

Commit

Permalink
REGRESSION(264248@main): A floating element can cause a list item's b…
Browse files Browse the repository at this point in the history
…ullet to be orphaned on overconstrained lines

https://bugs.webkit.org/show_bug.cgi?id=266801
<rdar://problem/120022893>

Reviewed by Antti Koivisto.

Do not disconnect list marker and the associated list item when they are embedded in a list element (e.g. <div><ul><li> vs. <div><li>) -and they are not positioned inside.
This quirky behavior matches both Chrome and legacy line layout.

* LayoutTests/fast/inline/list-item-inside-list-and-wrap-expected.html: Added.
* LayoutTests/fast/inline/list-item-inside-list-and-wrap.html: Added.
* Source/WebCore/layout/formattingContexts/inline/InlineFormattingUtils.cpp:
(WebCore::Layout::isAtSoftWrapOpportunity): Check whether we are inside a list element to decide whether we can break after the list marker.
* Source/WebCore/layout/integration/LayoutIntegrationBoxTree.cpp:
(WebCore::LayoutIntegration::BoxTree::createLayoutBox):
* Source/WebCore/layout/layouttree/LayoutElementBox.h:
(WebCore::Layout::ElementBox::isListMarkerInsideList const):

Canonical link: https://commits.webkit.org/272451@main
  • Loading branch information
alanbaradlay committed Dec 22, 2023
1 parent 270a645 commit ba97ecf
Show file tree
Hide file tree
Showing 5 changed files with 57 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
<style>
.container {
width: 400px;
font-family: Monospace;
}
.float_box {
float: left;
width: 400px;
height: 20px;
}
</style>
<div class=container>
<ul style="margin-top: 28px;">
<li>Pass if bullet appears in front of item</li>
</ul>
<div class=container>
<div class=float_box style="width: 380px"></div>
<li><br>Pass if item wraps</li>
</div>
</div>
22 changes: 22 additions & 0 deletions LayoutTests/fast/inline/list-item-inside-list-and-wrap.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
<style>
.container {
width: 400px;
font-family: Monospace;
}

.float_box {
float: left;
width: 400px;
height: 20px;
}
</style>
<div class=container>
<div class=float_box></div>
<ul>
<li>Pass if bullet appears in front of item</li>
</ul>
</div>
<div class=container>
<div class=float_box style="width: 380px"></div>
<li>Pass if item wraps</li>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -404,8 +404,14 @@ static inline bool isAtSoftWrapOpportunity(const InlineItem& previous, const Inl
// [text-][text] : after [hyphen] position is a soft wrap opportunity.
return endsWithSoftWrapOpportunity(currentInlineTextItem, nextInlineTextItem);
}
if (previous.layoutBox().isListMarkerBox() || next.layoutBox().isListMarkerBox())
if (previous.layoutBox().isListMarkerBox()) {
auto& listMarkerBox = downcast<ElementBox>(previous.layoutBox());
return !listMarkerBox.isListMarkerInsideList() || !listMarkerBox.isListMarkerOutside();
}
if (next.layoutBox().isListMarkerBox()) {
// FIXME: SHould this ever be the case?
return true;
}
if (previous.isBox() || next.isBox()) {
// [text][inline box start][inline box end][inline box] (text<span></span><img>) : there's a soft wrap opportunity between the [text] and [img].
// The line breaking behavior of a replaced element or other atomic inline is equivalent to an ideographic character.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -231,6 +231,8 @@ UniqueRef<Layout::Box> BoxTree::createLayoutBox(RenderObject& renderer)
listMarkerAttributes.add(Layout::ElementBox::ListMarkerAttribute::Image);
if (!listMarkerRenderer.isInside())
listMarkerAttributes.add(Layout::ElementBox::ListMarkerAttribute::Outside);
if (listMarkerRenderer.listItem() && !listMarkerRenderer.listItem()->notInList())
listMarkerAttributes.add(Layout::ElementBox::ListMarkerAttribute::HasListElementAncestor);
return makeUniqueRef<Layout::ElementBox>(elementAttributes(renderElement), listMarkerAttributes, WTFMove(style), WTFMove(firstLineStyle));
}

Expand Down
7 changes: 6 additions & 1 deletion Source/WebCore/layout/layouttree/LayoutElementBox.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,11 @@ class ElementBox : public Box {
public:
ElementBox(ElementAttributes&&, RenderStyle&&, std::unique_ptr<RenderStyle>&& firstLineStyle = nullptr, OptionSet<BaseTypeFlag> = { ElementBoxFlag });

enum class ListMarkerAttribute : uint8_t { Image = 1 << 0, Outside = 1 << 1 };
enum class ListMarkerAttribute : uint8_t {
Image = 1 << 0,
Outside = 1 << 1,
HasListElementAncestor = 1 << 2
};
ElementBox(ElementAttributes&&, OptionSet<ListMarkerAttribute>, RenderStyle&&, std::unique_ptr<RenderStyle>&& firstLineStyle = nullptr);

struct ReplacedAttributes {
Expand Down Expand Up @@ -85,6 +89,7 @@ class ElementBox : public Box {

bool isListMarkerImage() const { return m_replacedData && m_replacedData->listMarkerAttributes.contains(ListMarkerAttribute::Image); }
bool isListMarkerOutside() const { return m_replacedData && m_replacedData->listMarkerAttributes.contains(ListMarkerAttribute::Outside); }
bool isListMarkerInsideList() const { return m_replacedData && m_replacedData->listMarkerAttributes.contains(ListMarkerAttribute::HasListElementAncestor); }

// FIXME: This doesn't belong.
CachedImage* cachedImage() const { return m_replacedData ? m_replacedData->cachedImage : nullptr; }
Expand Down

0 comments on commit ba97ecf

Please sign in to comment.