Skip to content

Commit

Permalink
[LFC][IFC] Empty generated content should not prevent margin collapsing
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=243485

Reviewed by Antti Koivisto.

In some cases (e.g. pseudo content with '') we construct empty InlineTextItems (they are required so that we keep layout boxes and inline items synced). These truly empty inline items should be collapsed at InlineLine::append so that they don't show up on the line box as (blank) text runs (and make the iterator believe there's content on the line, which in turn prevents margin collapsing).
(This makes content: ''; behave as content: ' '; (provided collapsing is allowed))

* LayoutTests/fast/inline/incorrect-content-height-with-empty-generated-content-expected.html: Added.
* LayoutTests/fast/inline/incorrect-content-height-with-empty-generated-content.html: Added.
* Source/WebCore/layout/formattingContexts/inline/InlineLine.cpp:
(WebCore::Layout::Line::appendTextContent):
* Source/WebCore/layout/formattingContexts/inline/InlineTextItem.cpp:
(WebCore::Layout::InlineTextItem::InlineTextItem):
* Source/WebCore/layout/formattingContexts/inline/InlineTextItem.h:
(WebCore::Layout::InlineTextItem::isEmpty const):
(WebCore::Layout::InlineTextItem::createEmptyItem):

Canonical link: https://commits.webkit.org/253079@main
  • Loading branch information
alanbaradlay committed Aug 3, 2022
1 parent febc2b3 commit fe9010d
Show file tree
Hide file tree
Showing 15 changed files with 55 additions and 42 deletions.
@@ -0,0 +1,9 @@
<style>
div {
width: 100px;
height: 100px;
background-color: green;
padding: 1px;
}
</style>
<div></div>
@@ -0,0 +1,16 @@
<style>
div {
width: 100px;
background-color: green;
padding: 1px;
}

span::before {
display: block;
content: '';
margin-top: 100px;
margin-bottom: 100px;
}
</style>
<!-- empty content should not prevent margin collapsing -->
<div><span></span></div>
Expand Up @@ -7,5 +7,4 @@ layer at (8,8) size 784x100
RenderBlock {DIV} at (0,0) size 784x100
layer at (8,8) size 100x100
RenderBlock (generated) at (0,0) size 100x100 [bgcolor=#008000]
RenderText at (0,-14) size 0x18
text run at (0,-14) width 0: ""
RenderText at (0,0) size 0x0
Expand Up @@ -11,5 +11,4 @@ layer at (0,0) size 800x16
RenderTableSection (anonymous) at (0,0) size 0x0
RenderTableRow (anonymous) at (0,0) size 0x0
RenderTableCell (anonymous) at (0,0) size 0x0 [r=0 c=0 rs=1 cs=1]
RenderText at (0,-40) size 0x50
text run at (0,-40) width 0: ""
RenderText at (0,0) size 0x0
6 changes: 2 additions & 4 deletions LayoutTests/platform/ios/fast/css/acid2-expected.txt
Expand Up @@ -20,11 +20,9 @@ layer at (36,2640) size 764x226
RenderBlock {DIV} at (12,0) size 144x48 [bgcolor=#FFFF00]
RenderBlock {DIV} at (60,12) size 24x24 [bgcolor=#FF0000]
RenderBlock (generated) at (0,0) size 24x12 [border: none (12px solid #FFFF00) (12px solid #000000) (12px solid #FFFF00)]
RenderText at (12,-12) size 0x15
text run at (12,-12) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock (generated) at (0,12) size 24x12 [border: (12px solid #000000) (12px solid #FFFF00) none (12px solid #FFFF00)]
RenderText at (12,0) size 0x15
text run at (12,0) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock {DIV} at (87,75) size 590x0
RenderBlock {DIV} at (48,0) size 518x0
RenderBlock {DIV} at (48,132) size 668x24
Expand Down
6 changes: 2 additions & 4 deletions LayoutTests/platform/ios/fast/css/acid2-pixel-expected.txt
Expand Up @@ -20,11 +20,9 @@ layer at (36,72) size 764x226
RenderBlock {DIV} at (12,0) size 144x48 [bgcolor=#FFFF00]
RenderBlock {DIV} at (60,12) size 24x24 [bgcolor=#FF0000]
RenderBlock (generated) at (0,0) size 24x12 [border: none (12px solid #FFFF00) (12px solid #000000) (12px solid #FFFF00)]
RenderText at (12,-12) size 0x15
text run at (12,-12) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock (generated) at (0,12) size 24x12 [border: (12px solid #000000) (12px solid #FFFF00) none (12px solid #FFFF00)]
RenderText at (12,0) size 0x15
text run at (12,0) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock {DIV} at (87,75) size 590x0
RenderBlock {DIV} at (48,0) size 518x0
RenderBlock {DIV} at (48,132) size 668x24
Expand Down
6 changes: 2 additions & 4 deletions LayoutTests/platform/ios/http/tests/misc/acid2-expected.txt
Expand Up @@ -20,11 +20,9 @@ layer at (36,2640) size 764x226
RenderBlock {DIV} at (12,0) size 144x48 [bgcolor=#FFFF00]
RenderBlock {DIV} at (60,12) size 24x24 [bgcolor=#FF0000]
RenderBlock (generated) at (0,0) size 24x12 [border: none (12px solid #FFFF00) (12px solid #000000) (12px solid #FFFF00)]
RenderText at (12,-12) size 0x15
text run at (12,-12) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock (generated) at (0,12) size 24x12 [border: (12px solid #000000) (12px solid #FFFF00) none (12px solid #FFFF00)]
RenderText at (12,0) size 0x15
text run at (12,0) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock {DIV} at (87,75) size 590x0
RenderBlock {DIV} at (48,0) size 518x0
RenderBlock {DIV} at (48,132) size 668x24
Expand Down
Expand Up @@ -20,11 +20,9 @@ layer at (36,72) size 764x226
RenderBlock {DIV} at (12,0) size 144x48 [bgcolor=#FFFF00]
RenderBlock {DIV} at (60,12) size 24x24 [bgcolor=#FF0000]
RenderBlock (generated) at (0,0) size 24x12 [border: none (12px solid #FFFF00) (12px solid #000000) (12px solid #FFFF00)]
RenderText at (12,-12) size 0x15
text run at (12,-12) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock (generated) at (0,12) size 24x12 [border: (12px solid #000000) (12px solid #FFFF00) none (12px solid #FFFF00)]
RenderText at (12,0) size 0x15
text run at (12,0) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock {DIV} at (87,75) size 590x0
RenderBlock {DIV} at (48,0) size 518x0
RenderBlock {DIV} at (48,132) size 668x24
Expand Down
6 changes: 2 additions & 4 deletions LayoutTests/platform/mac/fast/css/acid2-expected.txt
Expand Up @@ -20,11 +20,9 @@ layer at (36,2638) size 764x226
RenderBlock {DIV} at (12,0) size 144x48 [bgcolor=#FFFF00]
RenderBlock {DIV} at (60,12) size 24x24 [bgcolor=#FF0000]
RenderBlock (generated) at (0,0) size 24x12 [border: none (12px solid #FFFF00) (12px solid #000000) (12px solid #FFFF00)]
RenderText at (12,-11) size 0x14
text run at (12,-11) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock (generated) at (0,12) size 24x12 [border: (12px solid #000000) (12px solid #FFFF00) none (12px solid #FFFF00)]
RenderText at (12,1) size 0x14
text run at (12,1) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock {DIV} at (87,75) size 590x0
RenderBlock {DIV} at (48,0) size 518x0
RenderBlock {DIV} at (48,132) size 668x24
Expand Down
6 changes: 2 additions & 4 deletions LayoutTests/platform/mac/fast/css/acid2-pixel-expected.txt
Expand Up @@ -20,11 +20,9 @@ layer at (36,72) size 764x226
RenderBlock {DIV} at (12,0) size 144x48 [bgcolor=#FFFF00]
RenderBlock {DIV} at (60,12) size 24x24 [bgcolor=#FF0000]
RenderBlock (generated) at (0,0) size 24x12 [border: none (12px solid #FFFF00) (12px solid #000000) (12px solid #FFFF00)]
RenderText at (12,-11) size 0x14
text run at (12,-11) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock (generated) at (0,12) size 24x12 [border: (12px solid #000000) (12px solid #FFFF00) none (12px solid #FFFF00)]
RenderText at (12,1) size 0x14
text run at (12,1) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock {DIV} at (87,75) size 590x0
RenderBlock {DIV} at (48,0) size 518x0
RenderBlock {DIV} at (48,132) size 668x24
Expand Down
6 changes: 2 additions & 4 deletions LayoutTests/platform/mac/http/tests/misc/acid2-expected.txt
Expand Up @@ -20,11 +20,9 @@ layer at (36,2638) size 764x226
RenderBlock {DIV} at (12,0) size 144x48 [bgcolor=#FFFF00]
RenderBlock {DIV} at (60,12) size 24x24 [bgcolor=#FF0000]
RenderBlock (generated) at (0,0) size 24x12 [border: none (12px solid #FFFF00) (12px solid #000000) (12px solid #FFFF00)]
RenderText at (12,-11) size 0x14
text run at (12,-11) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock (generated) at (0,12) size 24x12 [border: (12px solid #000000) (12px solid #FFFF00) none (12px solid #FFFF00)]
RenderText at (12,1) size 0x14
text run at (12,1) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock {DIV} at (87,75) size 590x0
RenderBlock {DIV} at (48,0) size 518x0
RenderBlock {DIV} at (48,132) size 668x24
Expand Down
Expand Up @@ -20,11 +20,9 @@ layer at (36,72) size 764x226
RenderBlock {DIV} at (12,0) size 144x48 [bgcolor=#FFFF00]
RenderBlock {DIV} at (60,12) size 24x24 [bgcolor=#FF0000]
RenderBlock (generated) at (0,0) size 24x12 [border: none (12px solid #FFFF00) (12px solid #000000) (12px solid #FFFF00)]
RenderText at (12,-11) size 0x14
text run at (12,-11) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock (generated) at (0,12) size 24x12 [border: (12px solid #000000) (12px solid #FFFF00) none (12px solid #FFFF00)]
RenderText at (12,1) size 0x14
text run at (12,1) width 0: ""
RenderText at (0,0) size 0x0
RenderBlock {DIV} at (87,75) size 590x0
RenderBlock {DIV} at (48,0) size 518x0
RenderBlock {DIV} at (48,132) size 668x24
Expand Down
Expand Up @@ -293,6 +293,10 @@ void Line::appendInlineBoxEnd(const InlineItem& inlineItem, const RenderStyle& s
void Line::appendTextContent(const InlineTextItem& inlineTextItem, const RenderStyle& style, InlineLayoutUnit logicalWidth)
{
auto willCollapseCompletely = [&] {
if (m_runs.isEmpty() && inlineTextItem.isEmpty()) {
// Some generated content initiates empty text items. They are truly collapsible.
return true;
}
if (!inlineTextItem.isWhitespace()) {
auto isLeadingCollapsibleNonBreakingSpace = [&] {
// Let's check for leading non-breaking space collapsing to match legacy line layout quirk.
Expand Down
Expand Up @@ -50,8 +50,8 @@ InlineTextItem::InlineTextItem(const InlineTextBox& inlineTextBox, unsigned star
m_textItemType = textItemType;
}

InlineTextItem::InlineTextItem(const InlineTextBox& inlineTextBox, UBiDiLevel bidiLevel)
: InlineItem(inlineTextBox, Type::Text, bidiLevel)
InlineTextItem::InlineTextItem(const InlineTextBox& inlineTextBox)
: InlineItem(inlineTextBox, Type::Text, UBIDI_DEFAULT_LTR)
{
}

Expand Down
10 changes: 6 additions & 4 deletions Source/WebCore/layout/formattingContexts/inline/InlineTextItem.h
Expand Up @@ -41,12 +41,13 @@ class InlineTextItem : public InlineItem {
public:
static InlineTextItem createWhitespaceItem(const InlineTextBox&, unsigned start, unsigned length, UBiDiLevel, bool isWordSeparator, std::optional<InlineLayoutUnit> width);
static InlineTextItem createNonWhitespaceItem(const InlineTextBox&, unsigned start, unsigned length, UBiDiLevel, bool hasTrailingSoftHyphen, std::optional<InlineLayoutUnit> width);
static InlineTextItem createEmptyItem(const InlineTextBox&, UBiDiLevel = UBIDI_DEFAULT_LTR);
static InlineTextItem createEmptyItem(const InlineTextBox&);

unsigned start() const { return m_startOrPosition; }
unsigned end() const { return start() + length(); }
unsigned length() const { return m_length; }

bool isEmpty() const { return !length() && m_textItemType == TextItemType::Undefined; }
bool isWhitespace() const { return m_textItemType == TextItemType::Whitespace; }
bool isWordSeparator() const { return m_isWordSeparator; }
bool isZeroWidthSpaceSeparator() const;
Expand All @@ -68,7 +69,7 @@ class InlineTextItem : public InlineItem {
InlineTextItem split(size_t leftSideLength);

InlineTextItem(const InlineTextBox&, unsigned start, unsigned length, UBiDiLevel, bool hasTrailingSoftHyphen, bool isWordSeparator, std::optional<InlineLayoutUnit> width, TextItemType);
explicit InlineTextItem(const InlineTextBox&, UBiDiLevel);
explicit InlineTextItem(const InlineTextBox&);
};

inline InlineTextItem InlineTextItem::createWhitespaceItem(const InlineTextBox& inlineTextBox, unsigned start, unsigned length, UBiDiLevel bidiLevel, bool isWordSeparator, std::optional<InlineLayoutUnit> width)
Expand All @@ -82,9 +83,10 @@ inline InlineTextItem InlineTextItem::createNonWhitespaceItem(const InlineTextBo
return { inlineTextBox, start, length, bidiLevel, hasTrailingSoftHyphen, false, width, TextItemType::NonWhitespace };
}

inline InlineTextItem InlineTextItem::createEmptyItem(const InlineTextBox& inlineTextBox, UBiDiLevel bidiLevel)
inline InlineTextItem InlineTextItem::createEmptyItem(const InlineTextBox& inlineTextBox)
{
return InlineTextItem { inlineTextBox, bidiLevel };
ASSERT(!inlineTextBox.content().length());
return InlineTextItem { inlineTextBox };
}

}
Expand Down

0 comments on commit fe9010d

Please sign in to comment.