Commit 5f491fd1 authored by Koji Ishii's avatar Koji Ishii Committed by Commit Bot

[LayoutNG] Fix vertical-align to use parent's font

This patch fixes 'vertical-align' values that need font metrics:
  vertical-align: sub
  vertical-align: super
  vertical-align: middle
to use parent's font instead of the box that has 'vertical-align'
applied.

The CSS2 'vertical-align'[1] defines to use parent's font for
the 'middle' value, and matches to our current code.

It looks like no CSS2 test suites test this condition, but some
tests in css-writing-modes do.

Rebaselines look to be more correct.

[1] https://drafts.csswg.org/css2/visudet.html#propdef-vertical-align

Bug: 636993
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Iaaa7448687173a9ecf0c39b55914a0fad4834318
Reviewed-on: https://chromium-review.googlesource.com/1069955
Commit-Queue: Koji Ishii <kojii@chromium.org>
Reviewed-by: default avatarEmil A Eklund <eae@chromium.org>
Cr-Commit-Position: refs/heads/master@{#561586}
parent 372aac60
...@@ -458,9 +458,6 @@ crbug.com/591099 external/wpt/css/css-writing-modes/vertical-alignment-007.xht [ ...@@ -458,9 +458,6 @@ crbug.com/591099 external/wpt/css/css-writing-modes/vertical-alignment-007.xht [
crbug.com/591099 external/wpt/css/css-writing-modes/vertical-alignment-vlr-023.xht [ Failure ] crbug.com/591099 external/wpt/css/css-writing-modes/vertical-alignment-vlr-023.xht [ Failure ]
crbug.com/591099 external/wpt/css/css-writing-modes/vertical-alignment-vlr-025.xht [ Failure ] crbug.com/591099 external/wpt/css/css-writing-modes/vertical-alignment-vlr-025.xht [ Failure ]
crbug.com/591099 external/wpt/css/css-writing-modes/vertical-alignment-vlr-027.xht [ Failure ] crbug.com/591099 external/wpt/css/css-writing-modes/vertical-alignment-vlr-027.xht [ Failure ]
crbug.com/591099 external/wpt/css/css-writing-modes/vertical-alignment-vrl-022.xht [ Failure ]
crbug.com/591099 external/wpt/css/css-writing-modes/vertical-alignment-vrl-024.xht [ Failure ]
crbug.com/591099 external/wpt/css/css-writing-modes/vertical-alignment-vrl-026.xht [ Failure ]
crbug.com/591099 external/wpt/css/css-writing-modes/wm-propagation-body-006.xht [ Failure ] crbug.com/591099 external/wpt/css/css-writing-modes/wm-propagation-body-006.xht [ Failure ]
crbug.com/591099 external/wpt/css/geometry/interfaces.worker.html [ Pass ] crbug.com/591099 external/wpt/css/geometry/interfaces.worker.html [ Pass ]
crbug.com/591099 external/wpt/css/selectors/focus-within-004.html [ Pass ] crbug.com/591099 external/wpt/css/selectors/focus-within-004.html [ Pass ]
......
layer at (0,0) size 800x600 layer at (0,0) size 800x600
LayoutView at (0,0) size 800x600 LayoutView at (0,0) size 800x600
layer at (0,0) size 800x307 layer at (0,0) size 800x307
LayoutNGBlockFlow {HTML} at (0,0) size 800x306.66 LayoutNGBlockFlow {HTML} at (0,0) size 800x307.19
LayoutNGBlockFlow {BODY} at (8,16) size 784x274.66 LayoutNGBlockFlow {BODY} at (8,16) size 784x275.19
LayoutNGBlockFlow {DIV} at (0,0) size 784x274.66 LayoutNGBlockFlow {DIV} at (0,0) size 784x275.19
LayoutNGBlockFlow {P} at (0,0) size 784x40 LayoutNGBlockFlow {P} at (0,0) size 784x40
LayoutText {#text} at (0,0) size 749x19 LayoutText {#text} at (0,0) size 749x19
text run at (0,0) width 749: "The background color of this paragraph should turn to green when the mouse pointer hovers over any of the following:" text run at (0,0) width 749: "The background color of this paragraph should turn to green when the mouse pointer hovers over any of the following:"
...@@ -33,10 +33,10 @@ layer at (0,0) size 800x307 ...@@ -33,10 +33,10 @@ layer at (0,0) size 800x307
LayoutNGTableCell {TD} at (2,84) size 107x22 [r=1 c=0 rs=1 cs=1] LayoutNGTableCell {TD} at (2,84) size 107x22 [r=1 c=0 rs=1 cs=1]
LayoutText {#text} at (1,1) size 59x19 LayoutText {#text} at (1,1) size 59x19
text run at (1,1) width 59: "This text." text run at (1,1) width 59: "This text."
LayoutNGBlockFlow {P} at (0,216) size 784x22.66 LayoutNGBlockFlow {P} at (0,216) size 784x23.19
LayoutInline {SUB} at (0,0) size 51x16 LayoutInline {SUB} at (0,0) size 51x16
LayoutText {#text} at (0,6) size 51x16 LayoutText {#text} at (0,7) size 51x16
text run at (0,6) width 51: "This text." text run at (0,7) width 51: "This text."
LayoutNGBlockFlow {P} at (0,254.66) size 784x20 LayoutNGBlockFlow {P} at (0,255.19) size 784x20
LayoutText {#text} at (0,0) size 573x19 LayoutText {#text} at (0,0) size 573x19
text run at (0,0) width 573: "...and anything else between the top of the first paragraph and the bottom of this paragraph." text run at (0,0) width 573: "...and anything else between the top of the first paragraph and the bottom of this paragraph."
layer at (0,0) size 800x600 layer at (0,0) size 800x600
LayoutView at (0,0) size 800x600 LayoutView at (0,0) size 800x600
layer at (0,0) size 800x307 layer at (0,0) size 800x307
LayoutNGBlockFlow {html} at (0,0) size 800x306.66 LayoutNGBlockFlow {html} at (0,0) size 800x307.19
LayoutNGBlockFlow {body} at (8,16) size 784x274.66 LayoutNGBlockFlow {body} at (8,16) size 784x275.19
LayoutNGBlockFlow {div} at (0,0) size 784x274.66 LayoutNGBlockFlow {div} at (0,0) size 784x275.19
LayoutNGBlockFlow {p} at (0,0) size 784x40 LayoutNGBlockFlow {p} at (0,0) size 784x40
LayoutText {#text} at (0,0) size 749x19 LayoutText {#text} at (0,0) size 749x19
text run at (0,0) width 749: "The background color of this paragraph should turn to green when the mouse pointer hovers over any of the following:" text run at (0,0) width 749: "The background color of this paragraph should turn to green when the mouse pointer hovers over any of the following:"
...@@ -33,10 +33,10 @@ layer at (0,0) size 800x307 ...@@ -33,10 +33,10 @@ layer at (0,0) size 800x307
LayoutNGTableCell {td} at (2,84) size 107x22 [r=1 c=0 rs=1 cs=1] LayoutNGTableCell {td} at (2,84) size 107x22 [r=1 c=0 rs=1 cs=1]
LayoutText {#text} at (1,1) size 59x19 LayoutText {#text} at (1,1) size 59x19
text run at (1,1) width 59: "This text." text run at (1,1) width 59: "This text."
LayoutNGBlockFlow {p} at (0,216) size 784x22.66 LayoutNGBlockFlow {p} at (0,216) size 784x23.19
LayoutInline {sub} at (0,0) size 51x16 LayoutInline {sub} at (0,0) size 51x16
LayoutText {#text} at (0,6) size 51x16 LayoutText {#text} at (0,7) size 51x16
text run at (0,6) width 51: "This text." text run at (0,7) width 51: "This text."
LayoutNGBlockFlow {p} at (0,254.66) size 784x20 LayoutNGBlockFlow {p} at (0,255.19) size 784x20
LayoutText {#text} at (0,0) size 573x19 LayoutText {#text} at (0,0) size 573x19
text run at (0,0) width 573: "...and anything else between the top of the first paragraph and the bottom of this paragraph." text run at (0,0) width 573: "...and anything else between the top of the first paragraph and the bottom of this paragraph."
layer at (0,0) size 800x600 layer at (0,0) size 800x600
LayoutView at (0,0) size 800x600 LayoutView at (0,0) size 800x600
layer at (0,0) size 800x307 layer at (0,0) size 800x307
LayoutNGBlockFlow {test} at (0,0) size 800x306.66 LayoutNGBlockFlow {test} at (0,0) size 800x307.19
LayoutNGBlockFlow {div} at (0,16) size 800x274.66 LayoutNGBlockFlow {div} at (0,16) size 800x275.19
LayoutNGBlockFlow {p} at (0,0) size 800x40 LayoutNGBlockFlow {p} at (0,0) size 800x40
LayoutText {#text} at (0,0) size 749x19 LayoutText {#text} at (0,0) size 749x19
text run at (0,0) width 749: "The background color of this paragraph should turn to green when the mouse pointer hovers over any of the following:" text run at (0,0) width 749: "The background color of this paragraph should turn to green when the mouse pointer hovers over any of the following:"
...@@ -32,10 +32,10 @@ layer at (0,0) size 800x307 ...@@ -32,10 +32,10 @@ layer at (0,0) size 800x307
LayoutNGTableCell {td} at (2,84) size 107x22 [r=1 c=0 rs=1 cs=1] LayoutNGTableCell {td} at (2,84) size 107x22 [r=1 c=0 rs=1 cs=1]
LayoutText {#text} at (1,1) size 59x19 LayoutText {#text} at (1,1) size 59x19
text run at (1,1) width 59: "This text." text run at (1,1) width 59: "This text."
LayoutNGBlockFlow {p} at (0,216) size 800x22.66 LayoutNGBlockFlow {p} at (0,216) size 800x23.19
LayoutInline {sub} at (0,0) size 51x16 LayoutInline {sub} at (0,0) size 51x16
LayoutText {#text} at (0,6) size 51x16 LayoutText {#text} at (0,7) size 51x16
text run at (0,6) width 51: "This text." text run at (0,7) width 51: "This text."
LayoutNGBlockFlow {p} at (0,254.66) size 800x20 LayoutNGBlockFlow {p} at (0,255.19) size 800x20
LayoutText {#text} at (0,0) size 573x19 LayoutText {#text} at (0,0) size 573x19
text run at (0,0) width 573: "...and anything else between the top of the first paragraph and the bottom of this paragraph." text run at (0,0) width 573: "...and anything else between the top of the first paragraph and the bottom of this paragraph."
layer at (0,0) size 800x600 layer at (0,0) size 800x600
LayoutView at (0,0) size 800x600 LayoutView at (0,0) size 800x600
layer at (0,0) size 800x307 layer at (0,0) size 800x307
LayoutNGBlockFlow {HTML} at (0,0) size 800x306.66 LayoutNGBlockFlow {HTML} at (0,0) size 800x307.19
LayoutNGBlockFlow {BODY} at (8,16) size 784x274.66 LayoutNGBlockFlow {BODY} at (8,16) size 784x275.19
LayoutNGBlockFlow {DIV} at (0,0) size 784x274.66 LayoutNGBlockFlow {DIV} at (0,0) size 784x275.19
LayoutNGBlockFlow {P} at (0,0) size 784x40 LayoutNGBlockFlow {P} at (0,0) size 784x40
LayoutText {#text} at (0,0) size 749x19 LayoutText {#text} at (0,0) size 749x19
text run at (0,0) width 749: "The background color of this paragraph should turn to green when the mouse pointer hovers over any of the following:" text run at (0,0) width 749: "The background color of this paragraph should turn to green when the mouse pointer hovers over any of the following:"
...@@ -33,10 +33,10 @@ layer at (0,0) size 800x307 ...@@ -33,10 +33,10 @@ layer at (0,0) size 800x307
LayoutNGTableCell {TD} at (2,84) size 107x22 [r=1 c=0 rs=1 cs=1] LayoutNGTableCell {TD} at (2,84) size 107x22 [r=1 c=0 rs=1 cs=1]
LayoutText {#text} at (1,1) size 59x19 LayoutText {#text} at (1,1) size 59x19
text run at (1,1) width 59: "This text." text run at (1,1) width 59: "This text."
LayoutNGBlockFlow {P} at (0,216) size 784x22.66 LayoutNGBlockFlow {P} at (0,216) size 784x23.19
LayoutInline {SUB} at (0,0) size 51x16 LayoutInline {SUB} at (0,0) size 51x16
LayoutText {#text} at (0,6) size 51x16 LayoutText {#text} at (0,7) size 51x16
text run at (0,6) width 51: "This text." text run at (0,7) width 51: "This text."
LayoutNGBlockFlow {P} at (0,254.66) size 784x20 LayoutNGBlockFlow {P} at (0,255.19) size 784x20
LayoutText {#text} at (0,0) size 573x19 LayoutText {#text} at (0,0) size 573x19
text run at (0,0) width 573: "...and anything else between the top of the first paragraph and the bottom of this paragraph." text run at (0,0) width 573: "...and anything else between the top of the first paragraph and the bottom of this paragraph."
...@@ -472,7 +472,9 @@ NGInlineLayoutStateStack::ApplyBaselineShift( ...@@ -472,7 +472,9 @@ NGInlineLayoutStateStack::ApplyBaselineShift(
NGInlineBoxState* box, NGInlineBoxState* box,
NGLineBoxFragmentBuilder::ChildList* line_box, NGLineBoxFragmentBuilder::ChildList* line_box,
FontBaseline baseline_type) { FontBaseline baseline_type) {
// Compute descendants that depend on the layout size of this box if any. // Some 'vertical-align' values require the size of their parents. Align all
// such descendant boxes that require the size of this box; they are queued in
// |pending_descendants|.
LayoutUnit baseline_shift; LayoutUnit baseline_shift;
if (!box->pending_descendants.IsEmpty()) { if (!box->pending_descendants.IsEmpty()) {
for (auto& child : box->pending_descendants) { for (auto& child : box->pending_descendants) {
...@@ -524,9 +526,15 @@ NGInlineLayoutStateStack::ApplyBaselineShift( ...@@ -524,9 +526,15 @@ NGInlineLayoutStateStack::ApplyBaselineShift(
if (vertical_align == EVerticalAlign::kBaseline) if (vertical_align == EVerticalAlign::kBaseline)
return kPositionNotPending; return kPositionNotPending;
// 'vertical-align' aplies only to inline-level elements. // 'vertical-align' aligns boxes relative to themselves, to their parent
// boxes, or to the line box, depends on the value.
// Because |box| is an item in |stack_|, |box[-1]| is its parent box.
// If this box doesn't have a parent; i.e., this box is a line box,
// 'vertical-align' has no effect.
DCHECK(box >= stack_.begin() && box < stack_.end());
if (box == stack_.begin()) if (box == stack_.begin())
return kPositionNotPending; return kPositionNotPending;
NGInlineBoxState& parent_box = box[-1];
// Check if there are any fragments to move. // Check if there are any fragments to move.
unsigned fragment_end = line_box->size(); unsigned fragment_end = line_box->size();
...@@ -535,10 +543,10 @@ NGInlineLayoutStateStack::ApplyBaselineShift( ...@@ -535,10 +543,10 @@ NGInlineLayoutStateStack::ApplyBaselineShift(
switch (vertical_align) { switch (vertical_align) {
case EVerticalAlign::kSub: case EVerticalAlign::kSub:
baseline_shift = style.ComputedFontSizeAsFixed() / 5 + 1; baseline_shift = parent_box.style->ComputedFontSizeAsFixed() / 5 + 1;
break; break;
case EVerticalAlign::kSuper: case EVerticalAlign::kSuper:
baseline_shift = -(style.ComputedFontSizeAsFixed() / 3 + 1); baseline_shift = -(parent_box.style->ComputedFontSizeAsFixed() / 3 + 1);
break; break;
case EVerticalAlign::kLength: { case EVerticalAlign::kLength: {
// 'Percentages: refer to the 'line-height' of the element itself'. // 'Percentages: refer to the 'line-height' of the element itself'.
...@@ -552,9 +560,10 @@ NGInlineLayoutStateStack::ApplyBaselineShift( ...@@ -552,9 +560,10 @@ NGInlineLayoutStateStack::ApplyBaselineShift(
} }
case EVerticalAlign::kMiddle: case EVerticalAlign::kMiddle:
baseline_shift = (box->metrics.ascent - box->metrics.descent) / 2; baseline_shift = (box->metrics.ascent - box->metrics.descent) / 2;
if (const SimpleFontData* font_data = style.GetFont().PrimaryFont()) { if (const SimpleFontData* parent_font_data =
parent_box.style->GetFont().PrimaryFont()) {
baseline_shift -= LayoutUnit::FromFloatRound( baseline_shift -= LayoutUnit::FromFloatRound(
font_data->GetFontMetrics().XHeight() / 2); parent_font_data->GetFontMetrics().XHeight() / 2);
} }
break; break;
case EVerticalAlign::kBaselineMiddle: case EVerticalAlign::kBaselineMiddle:
...@@ -568,8 +577,7 @@ NGInlineLayoutStateStack::ApplyBaselineShift( ...@@ -568,8 +577,7 @@ NGInlineLayoutStateStack::ApplyBaselineShift(
return kPositionPending; return kPositionPending;
default: default:
// Other values require the layout size of the parent box. // Other values require the layout size of the parent box.
SECURITY_CHECK(box != stack_.begin()); parent_box.pending_descendants.push_back(NGPendingPositions{
box[-1].pending_descendants.push_back(NGPendingPositions{
box->fragment_start, fragment_end, box->metrics, vertical_align}); box->fragment_start, fragment_end, box->metrics, vertical_align});
return kPositionPending; return kPositionPending;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment