Title: [271481] trunk/Source/WebCore
- Revision
- 271481
- Author
- [email protected]
- Date
- 2021-01-14 06:30:19 -0800 (Thu, 14 Jan 2021)
Log Message
[LFC][IFC][Quirk] Let's adopt a less quirky behavior for line breaks on non-empty lines
https://bugs.webkit.org/show_bug.cgi?id=220252
Reviewed by Antti Koivisto.
WebKit has this quirk behavior where <br> stretches the line box vertically only when the line is empty.
e.g.
<div><span><br></span></div>
We make the line as tall as the <br> is but
<div>some content<span style="font-size: 200px;"><br></span></div>
does not produce a ~200px tall line.
However when horizontal padding/border added, the line suddenly becomes ~200px tall:
<div>some content<span style="font-size: 200px; padding: 1px;"><br></span></div>
Removing this quirk makes the inlineLevelBoxAffectsLineBox logic simpler since we don't have to check against
whether the line is considered empty and the result is aligned with Chrome's behavior.
(This is in preparation for getting rid of the is-considered-empty concept for line/line box.)
* layout/inlineformatting/InlineFormattingContextGeometry.cpp:
(WebCore::Layout::LineBoxBuilder::constructInlineLevelBoxes):
* layout/inlineformatting/InlineFormattingContextQuirks.cpp:
(WebCore::Layout::InlineFormattingContext::Quirks::inlineLevelBoxAffectsLineBox const):
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (271480 => 271481)
--- trunk/Source/WebCore/ChangeLog 2021-01-14 11:02:43 UTC (rev 271480)
+++ trunk/Source/WebCore/ChangeLog 2021-01-14 14:30:19 UTC (rev 271481)
@@ -1,3 +1,31 @@
+2021-01-14 Zalan Bujtas <[email protected]>
+
+ [LFC][IFC][Quirk] Let's adopt a less quirky behavior for line breaks on non-empty lines
+ https://bugs.webkit.org/show_bug.cgi?id=220252
+
+ Reviewed by Antti Koivisto.
+
+ WebKit has this quirk behavior where <br> stretches the line box vertically only when the line is empty.
+ e.g.
+
+ <div><span><br></span></div>
+ We make the line as tall as the <br> is but
+
+ <div>some content<span style="font-size: 200px;"><br></span></div>
+ does not produce a ~200px tall line.
+
+ However when horizontal padding/border added, the line suddenly becomes ~200px tall:
+ <div>some content<span style="font-size: 200px; padding: 1px;"><br></span></div>
+
+ Removing this quirk makes the inlineLevelBoxAffectsLineBox logic simpler since we don't have to check against
+ whether the line is considered empty and the result is aligned with Chrome's behavior.
+ (This is in preparation for getting rid of the is-considered-empty concept for line/line box.)
+
+ * layout/inlineformatting/InlineFormattingContextGeometry.cpp:
+ (WebCore::Layout::LineBoxBuilder::constructInlineLevelBoxes):
+ * layout/inlineformatting/InlineFormattingContextQuirks.cpp:
+ (WebCore::Layout::InlineFormattingContext::Quirks::inlineLevelBoxAffectsLineBox const):
+
2021-01-14 Martin Robinson <[email protected]>
[css-scroll-snap] scroll-snap-align parsing is incorrect/backwards
Modified: trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextQuirks.cpp (271480 => 271481)
--- trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextQuirks.cpp 2021-01-14 11:02:43 UTC (rev 271480)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContextQuirks.cpp 2021-01-14 14:30:19 UTC (rev 271481)
@@ -44,6 +44,16 @@
bool InlineFormattingContext::Quirks::inlineLevelBoxAffectsLineBox(const LineBox::InlineLevelBox& inlineLevelBox, const LineBox& lineBox) const
{
+ if (inlineLevelBox.isLineBreakBox()) {
+ if (layoutState().inNoQuirksMode())
+ return true;
+ // In quirks mode linebreak boxes (<br>) affect the line box when they are inside a non-root inline box (<span></span>) or when
+ // the line has no other inline level box/root inlinebox has no content.
+ auto& parentInlineBox = lineBox.inlineLevelBoxForLayoutBox(inlineLevelBox.layoutBox().parent());
+ if (!parentInlineBox.isRootInlineBox())
+ return true;
+ return !parentInlineBox.hasContent() && lineBox.nonRootInlineLevelBoxes().size() == 1;
+ }
if (inlineLevelBox.isInlineBox()) {
// Inline boxes (e.g. root inline box or <span>) affects line boxes either through the strut or actual content.
if (inlineLevelBox.hasContent())
@@ -58,11 +68,6 @@
auto inlineBoxHasImaginaryStrut = layoutState().inNoQuirksMode();
return inlineBoxHasImaginaryStrut && !lineBox.isConsideredEmpty();
}
- if (inlineLevelBox.isLineBreakBox()) {
- // <br> in non-standard mode stretches the line box only when the line is empty.
- // e.g. <div><span><br></span></div> will stretch but <div>this will not stretch to 200px<span style="font-size: 200px;"><br></span></div>
- return layoutState().inNoQuirksMode() ? true : lineBox.isConsideredEmpty();
- }
if (inlineLevelBox.isAtomicInlineLevelBox()) {
if (inlineLevelBox.layoutBounds().height())
return true;
Modified: trunk/Source/WebCore/rendering/InlineFlowBox.cpp (271480 => 271481)
--- trunk/Source/WebCore/rendering/InlineFlowBox.cpp 2021-01-14 11:02:43 UTC (rev 271480)
+++ trunk/Source/WebCore/rendering/InlineFlowBox.cpp 2021-01-14 14:30:19 UTC (rev 271481)
@@ -137,7 +137,9 @@
if (child->renderer().isLineBreak()) {
// FIXME: This isn't ideal. We only turn off because current layout test results expect the <br> to be 0-height on the baseline.
// Other than making a zillion tests have to regenerate results, there's no reason to ditch the optimization here.
- shouldClearDescendantsHaveSameLineHeightAndBaseline = child->renderer().isBR();
+ auto childIsHardLinebreak = child->renderer().isBR();
+ shouldClearDescendantsHaveSameLineHeightAndBaseline = childIsHardLinebreak;
+ m_hasHardLinebreak = m_hasHardLinebreak || childIsHardLinebreak;
} else {
auto& childFlowBox = downcast<InlineFlowBox>(*child);
// Check the child's bit, and then also check for differences in font, line-height, vertical-align
@@ -595,8 +597,12 @@
} else if (child->verticalAlign() == VerticalAlign::Bottom && verticalAlignApplies(child->renderer())) {
if (maxPositionBottom < boxHeight)
maxPositionBottom = boxHeight;
- } else if (!inlineFlowBox || strictMode || inlineFlowBox->hasTextChildren() || (inlineFlowBox->descendantsHaveSameLineHeightAndBaseline() && inlineFlowBox->hasTextDescendants())
- || inlineFlowBox->renderer().hasInlineDirectionBordersOrPadding()) {
+ } else if (strictMode
+ || !inlineFlowBox
+ || inlineFlowBox->hasTextChildren()
+ || (inlineFlowBox->descendantsHaveSameLineHeightAndBaseline() && inlineFlowBox->hasTextDescendants())
+ || inlineFlowBox->renderer().hasInlineDirectionBordersOrPadding()
+ || inlineFlowBox->hasHardLinebreak()) {
// Note that these values can be negative. Even though we only affect the maxAscent and maxDescent values
// if our box (excluding line-height) was above (for ascent) or below (for descent) the root baseline, once you factor in line-height
// the final box can end up being fully above or fully below the root box's baseline! This is ok, but what it
Modified: trunk/Source/WebCore/rendering/InlineFlowBox.h (271480 => 271481)
--- trunk/Source/WebCore/rendering/InlineFlowBox.h 2021-01-14 11:02:43 UTC (rev 271480)
+++ trunk/Source/WebCore/rendering/InlineFlowBox.h 2021-01-14 14:30:19 UTC (rev 271481)
@@ -44,6 +44,7 @@
: InlineBox(renderer)
, m_includeLogicalLeftEdge(false)
, m_includeLogicalRightEdge(false)
+ , m_hasHardLinebreak(false)
, m_descendantsHaveSameLineHeightAndBaseline(true)
, m_baselineType(AlphabeticBaseline)
, m_hasAnnotationsBefore(false)
@@ -208,6 +209,7 @@
bool hasTextChildren() const { return m_hasTextChildren; }
bool hasTextDescendants() const { return m_hasTextDescendants; }
+ bool hasHardLinebreak() const { return m_hasHardLinebreak; }
void setHasTextChildren() { m_hasTextChildren = true; setHasTextDescendants(); }
void setHasTextDescendants() { m_hasTextDescendants = true; }
@@ -312,6 +314,7 @@
unsigned m_includeLogicalRightEdge : 1;
unsigned m_hasTextChildren : 1;
unsigned m_hasTextDescendants : 1;
+ unsigned m_hasHardLinebreak : 1;
unsigned m_descendantsHaveSameLineHeightAndBaseline : 1;
protected:
Modified: trunk/Source/WebCore/rendering/RenderInline.cpp (271480 => 271481)
--- trunk/Source/WebCore/rendering/RenderInline.cpp 2021-01-14 11:02:43 UTC (rev 271480)
+++ trunk/Source/WebCore/rendering/RenderInline.cpp 2021-01-14 14:30:19 UTC (rev 271481)
@@ -212,6 +212,7 @@
auto* parentStyle = &parent()->style();
RenderInline* parentRenderInline = is<RenderInline>(*parent()) ? downcast<RenderInline>(parent()) : nullptr;
+ auto hasHardLineBreakChildOnly = firstChild() && firstChild() == lastChild() && firstChild()->isBR();
bool checkFonts = document().inNoQuirksMode();
bool alwaysCreateLineBoxes = (parentRenderInline && parentRenderInline->alwaysCreateLineBoxes())
|| (parentRenderInline && parentStyle->verticalAlign() != VerticalAlign::Baseline)
@@ -218,7 +219,8 @@
|| style().verticalAlign() != VerticalAlign::Baseline
|| style().textEmphasisMark() != TextEmphasisMark::None
|| (checkFonts && (!parentStyle->fontCascade().fontMetrics().hasIdenticalAscentDescentAndLineGap(style().fontCascade().fontMetrics())
- || parentStyle->lineHeight() != style().lineHeight()));
+ || parentStyle->lineHeight() != style().lineHeight()))
+ || hasHardLineBreakChildOnly;
if (!alwaysCreateLineBoxes && checkFonts && view().usesFirstLineRules()) {
// Have to check the first line style as well.
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes
