[LayoutNG] Fix empty inlines to influence the used line height
This patch matches NGInlineLayoutAlgorithm to CSS2 10.8 Line height
calculations: the 'line-height' and 'vertical-align' properties[1]
defining empty inline elements should influence line height.
Also, since the introduction of NGInlineBoxState, we had two places to
compute the union of boxes; NGInlineBoxState.stack_[0] and
NGLineBoxFragmentBuilder. Some code were uniting boxes to wrong one.
This patch unifies them to NGInlineBoxState.
[1] https://drafts.csswg.org/css2/visudet.html#line-height
BUG=636993
Review-Url: https://codereview.chromium.org/2845493002
Cr-Commit-Position: refs/heads/master@{#467278}
diff --git a/third_party/WebKit/LayoutTests/TestExpectations b/third_party/WebKit/LayoutTests/TestExpectations
index ce194f76..faa6b25 100644
--- a/third_party/WebKit/LayoutTests/TestExpectations
+++ b/third_party/WebKit/LayoutTests/TestExpectations
@@ -826,6 +826,7 @@
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-006.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-008.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-009.xht [ Skip ]
+crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-011.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-012.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-013.xht [ Skip ]
crbug.com/635619 virtual/layout_ng/external/wpt/css/CSS2/normal-flow/inline-replaced-width-014.xht [ Skip ]
@@ -1287,7 +1288,6 @@
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/border-padding-bleed-002.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/border-padding-bleed-003.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/empty-inline-002.xht [ Skip ]
-crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/empty-inline-003.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-001.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-002.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/inline-formatting-context-003.xht [ Skip ]
@@ -1305,7 +1305,6 @@
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-002.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-004.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-005.xht [ Skip ]
-crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-applies-to-012.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-013.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-015.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-016.xht [ Skip ]
@@ -1335,7 +1334,6 @@
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-104.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-125.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-126.xht [ Skip ]
-crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-127.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-128.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-129.xht [ Skip ]
crbug.com/636993 virtual/layout_ng/external/wpt/css/CSS2/linebox/line-height-bleed-001.xht [ Skip ]
diff --git a/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_box_state.cc b/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_box_state.cc
index ffe34569..084904c0 100644
--- a/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_box_state.cc
+++ b/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_box_state.cc
@@ -11,9 +11,8 @@
namespace blink {
-void NGInlineBoxState::ComputeTextMetrics(const NGInlineItem& item,
+void NGInlineBoxState::ComputeTextMetrics(const ComputedStyle& style,
FontBaseline baseline_type) {
- const ComputedStyle& style = *item.Style();
text_metrics = NGLineHeightMetrics(style, baseline_type);
text_top = -text_metrics.ascent;
text_metrics.AddLeading(style.ComputedLineHeightAsFixed());
@@ -22,23 +21,47 @@
include_used_fonts = style.LineHeight().IsNegative();
}
+void NGInlineBoxState::AccumulateUsedFonts(const NGInlineItem& item,
+ unsigned start,
+ unsigned end,
+ FontBaseline baseline_type) {
+ HashSet<const SimpleFontData*> fallback_fonts;
+ item.GetFallbackFonts(&fallback_fonts, start, end);
+ for (const auto& fallback_font : fallback_fonts) {
+ NGLineHeightMetrics fallback_metrics(fallback_font->GetFontMetrics(),
+ baseline_type);
+ fallback_metrics.AddLeading(
+ fallback_font->GetFontMetrics().FixedLineSpacing());
+ metrics.Unite(fallback_metrics);
+ }
+}
+
NGInlineBoxState* NGInlineLayoutStateStack::OnBeginPlaceItems(
- const ComputedStyle* line_style) {
+ const ComputedStyle* line_style,
+ FontBaseline baseline_type) {
if (stack_.IsEmpty()) {
// For the first line, push a box state for the line itself.
stack_.Resize(1);
NGInlineBoxState* box = &stack_.back();
box->fragment_start = 0;
- box->style = line_style;
- return box;
+ } else {
+ // For the following lines, clear states that are not shared across lines.
+ for (auto& box : stack_) {
+ box.fragment_start = 0;
+ box.metrics = NGLineHeightMetrics();
+ DCHECK(box.pending_descendants.IsEmpty());
+ }
}
- // For the following lines, clear states that are not shared across lines.
- for (auto& box : stack_) {
- box.fragment_start = 0;
- box.metrics = NGLineHeightMetrics();
- DCHECK(box.pending_descendants.IsEmpty());
- }
+ // Initialize the box state for the line box.
+ NGInlineBoxState& line_box = LineBoxState();
+ line_box.style = line_style;
+
+ // Use a "strut" (a zero-width inline box with the element's font and
+ // line height properties) as the initial metrics for the line box.
+ // https://drafts.csswg.org/css2/visudet.html#strut
+ line_box.ComputeTextMetrics(*line_style, baseline_type);
+
return &stack_.back();
}
@@ -72,7 +95,9 @@
NGInlineBoxState* box = &(*it);
EndBoxState(box, line_box);
}
- line_box->UniteMetrics(stack_.front().metrics);
+
+ DCHECK(!LineBoxState().metrics.IsEmpty());
+ line_box->SetMetrics(LineBoxState().metrics);
}
void NGInlineLayoutStateStack::EndBoxState(NGInlineBoxState* box,
diff --git a/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_box_state.h b/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_box_state.h
index e3675e1..9a52e39 100644
--- a/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_box_state.h
+++ b/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_box_state.h
@@ -40,7 +40,11 @@
bool include_used_fonts = false;
// Compute text metrics for a box. All text in a box share the same metrics.
- void ComputeTextMetrics(const NGInlineItem&, FontBaseline);
+ void ComputeTextMetrics(const ComputedStyle& style, FontBaseline);
+ void AccumulateUsedFonts(const NGInlineItem&,
+ unsigned start,
+ unsigned end,
+ FontBaseline);
};
// Represents the inline tree structure. This class provides:
@@ -49,9 +53,12 @@
// 3) Cache common values for a box.
class NGInlineLayoutStateStack {
public:
+ // The box state for the line box.
+ NGInlineBoxState& LineBoxState() { return stack_.front(); }
+
// Initialize the box state stack for a new line.
// @return The initial box state for the line.
- NGInlineBoxState* OnBeginPlaceItems(const ComputedStyle*);
+ NGInlineBoxState* OnBeginPlaceItems(const ComputedStyle*, FontBaseline);
// Push a box state stack.
NGInlineBoxState* OnOpenTag(const NGInlineItem&,
diff --git a/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc b/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc
index 5e92c1ca..470da16 100644
--- a/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc
+++ b/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm.cc
@@ -394,35 +394,40 @@
const Vector<LineItemChunk, 32>& line_item_chunks) {
const Vector<NGInlineItem>& items = Node()->Items();
- // Use a "strut" (a zero-width inline box with the element's font and
- // line height properties) as the initial metrics for the line box.
- // https://drafts.csswg.org/css2/visudet.html#strut
const ComputedStyle& line_style = LineStyle();
NGLineHeightMetrics line_metrics(line_style, baseline_type_);
NGLineHeightMetrics line_metrics_with_leading = line_metrics;
line_metrics_with_leading.AddLeading(line_style.ComputedLineHeightAsFixed());
- NGLineBoxFragmentBuilder line_box(Node(), line_metrics_with_leading);
+ NGLineBoxFragmentBuilder line_box(Node());
// Compute heights of all inline items by placing the dominant baseline at 0.
// The baseline is adjusted after the height of the line box is computed.
NGTextFragmentBuilder text_builder(Node());
- NGInlineBoxState* box = box_states_.OnBeginPlaceItems(&LineStyle());
+ NGInlineBoxState* box =
+ box_states_.OnBeginPlaceItems(&LineStyle(), baseline_type_);
LayoutUnit inline_size;
for (const auto& line_item_chunk : line_item_chunks) {
const NGInlineItem& item = items[line_item_chunk.index];
LayoutUnit line_top;
if (item.Type() == NGInlineItem::kText) {
DCHECK(item.GetLayoutObject()->IsText());
- if (box->text_metrics.IsEmpty())
- box->ComputeTextMetrics(item, baseline_type_);
+ DCHECK(!box->text_metrics.IsEmpty());
line_top = box->text_top;
text_builder.SetSize(
{line_item_chunk.inline_size, box->text_metrics.LineHeight()});
// Take all used fonts into account if 'line-height: normal'.
- if (box->include_used_fonts)
- AccumulateUsedFonts(item, line_item_chunk, &line_box);
+ if (box->include_used_fonts) {
+ box->AccumulateUsedFonts(item, line_item_chunk.start_offset,
+ line_item_chunk.end_offset, baseline_type_);
+ }
} else if (item.Type() == NGInlineItem::kOpenTag) {
box = box_states_.OnOpenTag(item, &line_box, &text_builder);
+ // Compute text metrics for all inline boxes since even empty inlines
+ // influence the line height.
+ // https://drafts.csswg.org/css2/visudet.html#line-height
+ // TODO(kojii): Review if atomic inline level should have open/close.
+ if (!item.GetLayoutObject()->IsAtomicInlineLevel())
+ box->ComputeTextMetrics(*item.Style(), baseline_type_);
continue;
} else if (item.Type() == NGInlineItem::kCloseTag) {
box = box_states_.OnCloseTag(item, &line_box, box);
@@ -491,29 +496,15 @@
// the line box to the line top.
line_box.MoveChildrenInBlockDirection(baseline);
line_box.SetInlineSize(inline_size);
- container_builder_.AddChild(line_box.ToLineBoxFragment(),
- {LayoutUnit(), baseline - line_metrics.ascent});
+ container_builder_.AddChild(
+ line_box.ToLineBoxFragment(),
+ {LayoutUnit(), baseline + box_states_.LineBoxState().text_top});
max_inline_size_ = std::max(max_inline_size_, inline_size);
content_size_ = line_bottom;
return true;
}
-void NGInlineLayoutAlgorithm::AccumulateUsedFonts(
- const NGInlineItem& item,
- const LineItemChunk& line_item_chunk,
- NGLineBoxFragmentBuilder* line_box) {
- HashSet<const SimpleFontData*> fallback_fonts;
- item.GetFallbackFonts(&fallback_fonts, line_item_chunk.start_offset,
- line_item_chunk.end_offset);
- for (const auto& fallback_font : fallback_fonts) {
- NGLineHeightMetrics metrics(fallback_font->GetFontMetrics(),
- baseline_type_);
- metrics.AddLeading(fallback_font->GetFontMetrics().FixedLineSpacing());
- line_box->UniteMetrics(metrics);
- }
-}
-
LayoutUnit NGInlineLayoutAlgorithm::PlaceAtomicInline(
const NGInlineItem& item,
NGLineBoxFragmentBuilder* line_box,
diff --git a/third_party/WebKit/Source/core/layout/ng/inline/ng_line_box_fragment_builder.cc b/third_party/WebKit/Source/core/layout/ng/inline/ng_line_box_fragment_builder.cc
index baeab067..e972c2c 100644
--- a/third_party/WebKit/Source/core/layout/ng/inline/ng_line_box_fragment_builder.cc
+++ b/third_party/WebKit/Source/core/layout/ng/inline/ng_line_box_fragment_builder.cc
@@ -13,10 +13,8 @@
namespace blink {
-NGLineBoxFragmentBuilder::NGLineBoxFragmentBuilder(
- NGInlineNode* node,
- const NGLineHeightMetrics& metrics)
- : direction_(TextDirection::kLtr), node_(node), metrics_(metrics) {}
+NGLineBoxFragmentBuilder::NGLineBoxFragmentBuilder(NGInlineNode* node)
+ : direction_(TextDirection::kLtr), node_(node) {}
NGLineBoxFragmentBuilder& NGLineBoxFragmentBuilder::SetDirection(
TextDirection direction) {
@@ -51,9 +49,8 @@
offsets_[index].block_offset += delta;
}
-void NGLineBoxFragmentBuilder::UniteMetrics(
- const NGLineHeightMetrics& metrics) {
- metrics_.Unite(metrics);
+void NGLineBoxFragmentBuilder::SetMetrics(const NGLineHeightMetrics& metrics) {
+ metrics_ = metrics;
}
void NGLineBoxFragmentBuilder::SetBreakToken(
diff --git a/third_party/WebKit/Source/core/layout/ng/inline/ng_line_box_fragment_builder.h b/third_party/WebKit/Source/core/layout/ng/inline/ng_line_box_fragment_builder.h
index c1e0c89..e7e16de 100644
--- a/third_party/WebKit/Source/core/layout/ng/inline/ng_line_box_fragment_builder.h
+++ b/third_party/WebKit/Source/core/layout/ng/inline/ng_line_box_fragment_builder.h
@@ -20,7 +20,7 @@
STACK_ALLOCATED();
public:
- NGLineBoxFragmentBuilder(NGInlineNode*, const NGLineHeightMetrics&);
+ explicit NGLineBoxFragmentBuilder(NGInlineNode*);
NGLineBoxFragmentBuilder& SetDirection(TextDirection);
@@ -35,7 +35,7 @@
return children_;
}
- void UniteMetrics(const NGLineHeightMetrics&);
+ void SetMetrics(const NGLineHeightMetrics&);
const NGLineHeightMetrics& Metrics() const { return metrics_; }
// Set the break token for the fragment to build.
