Commit ddba7400 authored by Kent Tamura's avatar Kent Tamura Committed by Commit Bot

Fieldset NG: Fix block-direction position of LEGEND

According to the legacy layout behavior, WebKit behavior, and Firefox
behavior, margins of a LEGEND should not affect block-direction
position of the LEGEND. The border-box of the LEGEND should be centered
vertically in the top border of the FIELDSET.

- We should compare the block-start border width of the FIELDSET and
  the block size of the LEGEND border-box.

- Even if the block size of LEGEND border-box is smaller than the
  block-start border of the FIELDSET, block-end margin of the LEGEND
  can affect the position of the FIELDSET content box.

This fixes legend-after-margin-with-before-border-horizontal-mode.html
and legend-small-after-margin-before-border-horizontal-mode.html, and
the major part of legend-block-margins-2.html.

Test changes:
 * NGFieldsetLayoutAlgorithmTest.LegendBreakBeforeAvoid
  The test assumed a break happened in the block-start margin of a
  LEGEND. We ignore the block-start margin, and the test broke inside
  the LEGEND. Add 'break-inside:avoid' to avoid breaking inside the
  LEGEND.

 * NGFieldsetLayoutAlgorithmTest.MarginTopPastEndOfFragmentainer
  Removed. The test made sure that if a break happens inside the
  block-start margin of a LEGEND, the reaiming block-start margin was
  ignored.  We ignore the block-start margin, so we can't test the
  behavior.

Bug: 875235
Change-Id: I78db12c42d5d1d5a928b7871838a5330f4b6943a
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2331995
Commit-Queue: Kent Tamura <tkent@chromium.org>
Reviewed-by: default avatarMorten Stenshorne <mstensho@chromium.org>
Reviewed-by: default avatarAlison Maher <almaher@microsoft.com>
Cr-Commit-Position: refs/heads/master@{#794889}
parent 8be4b305
...@@ -270,7 +270,7 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutLegend( ...@@ -270,7 +270,7 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutLegend(
scoped_refptr<const NGLayoutResult> result; scoped_refptr<const NGLayoutResult> result;
scoped_refptr<const NGLayoutResult> previous_result; scoped_refptr<const NGLayoutResult> previous_result;
LayoutUnit block_offset = legend_margins.block_start; LayoutUnit block_offset;
do { do {
auto legend_space = CreateConstraintSpaceForLegend( auto legend_space = CreateConstraintSpaceForLegend(
legend, ChildAvailableSize(), percentage_size, block_offset); legend, ChildAvailableSize(), percentage_size, block_offset);
...@@ -296,19 +296,20 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutLegend( ...@@ -296,19 +296,20 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutLegend(
// We have already adjusted the legend block offset, no need to adjust // We have already adjusted the legend block offset, no need to adjust
// again. // again.
if (block_offset != legend_margins.block_start) { if (block_offset != LayoutUnit()) {
// If adjusting the block_offset caused the legend to break, revert back // If adjusting the block_offset caused the legend to break, revert back
// to the previous result. // to the previous result.
if (legend_broke_) { if (legend_broke_) {
result = std::move(previous_result); result = std::move(previous_result);
block_offset = legend_margins.block_start; block_offset = LayoutUnit();
} }
break; break;
} }
LayoutUnit legend_margin_box_block_size = LayoutUnit legend_border_box_block_size =
legend_margins.block_start +
NGFragment(writing_mode_, physical_fragment).BlockSize(); NGFragment(writing_mode_, physical_fragment).BlockSize();
LayoutUnit legend_margin_box_block_size =
legend_margins.block_start + legend_border_box_block_size;
LayoutUnit block_end_margin = legend_margins.block_end; LayoutUnit block_end_margin = legend_margins.block_end;
if (ConstraintSpace().HasKnownFragmentainerBlockSize()) { if (ConstraintSpace().HasKnownFragmentainerBlockSize()) {
...@@ -317,18 +318,16 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutLegend( ...@@ -317,18 +318,16 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutLegend(
} }
legend_margin_box_block_size += block_end_margin; legend_margin_box_block_size += block_end_margin;
LayoutUnit space_left = borders_.block_start - legend_margin_box_block_size; LayoutUnit space_left = borders_.block_start - legend_border_box_block_size;
if (space_left > LayoutUnit()) { if (space_left > LayoutUnit()) {
// Don't adjust the block-start offset of the legend if the legend broke. // Don't adjust the block-start offset of the legend if the legend broke.
if (legend_break_token || legend_broke_) if (legend_break_token || legend_broke_)
break; break;
// If the border is the larger one, though, it will stay put at the // https://html.spec.whatwg.org/C/#the-fieldset-and-legend-elements
// border-box block-start edge of the fieldset. Then it's the legend // * The element is expected to be positioned in the block-flow direction
// that needs to be pushed. We'll center the margin box in this case, to // such that its border box is centered over the border on the
// make sure that both margins remain within the area occupied by the // block-start side of the fieldset element.
// border also after adjustment.
block_offset += space_left / 2; block_offset += space_left / 2;
if (ConstraintSpace().HasBlockFragmentation()) { if (ConstraintSpace().HasBlockFragmentation()) {
// Save the previous result in case adjusting the block_offset causes // Save the previous result in case adjusting the block_offset causes
...@@ -336,23 +335,28 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutLegend( ...@@ -336,23 +335,28 @@ NGBreakStatus NGFieldsetLayoutAlgorithm::LayoutLegend(
previous_result = std::move(result); previous_result = std::move(result);
continue; continue;
} }
} else { }
// If the border is smaller, intrinsic_block_size_ should now be based on // If the border is smaller than the block end offset of the legend margin
// the size of the legend instead of the border. // box, intrinsic_block_size_ should now be based on the the block end
intrinsic_block_size_ = legend_margin_box_block_size; // offset of the legend margin box instead of the border.
LayoutUnit legend_margin_end_offset = block_offset +
legend_margin_box_block_size -
legend_margins.block_start;
if (legend_margin_end_offset > borders_.block_start) {
intrinsic_block_size_ = legend_margin_end_offset;
is_legend_past_border_ = true; is_legend_past_border_ = true;
// Don't adjust the block-start offset of the fragment border if it broke. // Don't adjust the block-start offset of the fragment border if it broke.
if (BreakToken() || (ConstraintSpace().HasKnownFragmentainerBlockSize() && if (BreakToken() || (ConstraintSpace().HasKnownFragmentainerBlockSize() &&
legend_margin_box_block_size > legend_margin_end_offset >
ConstraintSpace().FragmentainerBlockSize())) ConstraintSpace().FragmentainerBlockSize()))
break; break;
// If the legend is larger than the width of the fieldset block-start // If the legend is larger than the width of the fieldset block-start
// border, the actual padding edge of the fieldset will be moved // border, the actual padding edge of the fieldset will be moved
// accordingly. This will be the block-start offset for the fieldset // accordingly. This will be the block-start offset for the fieldset
// contents anonymous box. // contents anonymous box.
borders_.block_start = legend_margin_box_block_size; borders_.block_start = legend_margin_end_offset;
} }
break; break;
} while (true); } while (true);
......
...@@ -156,7 +156,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, SmallLegendLargeBorder) { ...@@ -156,7 +156,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, SmallLegendLargeBorder) {
String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::. String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:1000x200 offset:unplaced size:1000x200
offset:0,0 size:200x200 offset:0,0 size:200x200
offset:50,10 size:10x10 offset:50,15 size:10x10
offset:40,40 size:120x120 offset:40,40 size:120x120
offset:10,10 size:100x100 offset:10,10 size:100x100
)DUMP"; )DUMP";
...@@ -180,10 +180,10 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendOrthogonalWritingMode) { ...@@ -180,10 +180,10 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendOrthogonalWritingMode) {
String dump = DumpFragmentTree(GetElementById("container")); String dump = DumpFragmentTree(GetElementById("container"));
String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::. String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:1000x203 offset:unplaced size:1000x193
offset:0,0 size:126x203 offset:0,0 size:126x193
offset:43,10 size:10x50 offset:43,0 size:10x50
offset:3,80 size:120x120 offset:3,70 size:120x120
offset:10,10 size:100x100 offset:10,10 size:100x100
)DUMP"; )DUMP";
EXPECT_EQ(expectation, dump); EXPECT_EQ(expectation, dump);
...@@ -207,9 +207,9 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, VerticalLr) { ...@@ -207,9 +207,9 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, VerticalLr) {
String dump = DumpFragmentTree(GetElementById("container")); String dump = DumpFragmentTree(GetElementById("container"));
String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::. String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:1000x126 offset:unplaced size:1000x126
offset:0,0 size:178x126 offset:0,0 size:148x126
offset:30,23 size:10x50 offset:0,23 size:10x50
offset:55,3 size:120x120 offset:25,3 size:120x120
offset:10,10 size:100x100 offset:10,10 size:100x100
)DUMP"; )DUMP";
EXPECT_EQ(expectation, dump); EXPECT_EQ(expectation, dump);
...@@ -233,7 +233,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, VerticalRl) { ...@@ -233,7 +233,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, VerticalRl) {
String dump = DumpFragmentTree(GetElementById("container")); String dump = DumpFragmentTree(GetElementById("container"));
String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::. String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:1000x126 offset:unplaced size:1000x126
offset:0,0 size:178x126 offset:0,0 size:163x126
offset:153,23 size:10x50 offset:153,23 size:10x50
offset:3,3 size:120x120 offset:3,3 size:120x120
offset:10,10 size:100x100 offset:10,10 size:100x100
...@@ -1530,7 +1530,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendBreakBeforeAvoid) { ...@@ -1530,7 +1530,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendBreakBeforeAvoid) {
<div id="container"> <div id="container">
<div style="width:20px; height:90px;"></div> <div style="width:20px; height:90px;"></div>
<fieldset id="fieldset"> <fieldset id="fieldset">
<legend id="legend" style="break-before:avoid;"></legend> <legend id="legend" style="break-before:avoid; break-inside:avoid"></legend>
</fieldset> </fieldset>
</div> </div>
)HTML"); )HTML");
...@@ -1560,10 +1560,10 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendBreakBeforeAvoid) { ...@@ -1560,10 +1560,10 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendBreakBeforeAvoid) {
dump = DumpFragmentTree(fragment.get()); dump = DumpFragmentTree(fragment.get());
expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::. expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:1000x55 offset:unplaced size:1000x45
offset:0,0 size:120x55 offset:0,0 size:120x45
offset:20,10 size:10x25 offset:20,0 size:10x25
offset:10,45 size:100x0 offset:10,35 size:100x0
)DUMP"; )DUMP";
EXPECT_EQ(expectation, dump); EXPECT_EQ(expectation, dump);
} }
...@@ -1677,58 +1677,6 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendBreakAfterAvoid) { ...@@ -1677,58 +1677,6 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, LegendBreakAfterAvoid) {
EXPECT_EQ(expectation, dump); EXPECT_EQ(expectation, dump);
} }
TEST_F(NGFieldsetLayoutAlgorithmTest, MarginTopPastEndOfFragmentainer) {
// A block whose border box would start past the end of the current
// fragmentainer should start exactly at the start of the next fragmentainer,
// discarding what's left of the margin.
// https://www.w3.org/TR/css-break-3/#break-margins
SetBodyInnerHTML(R"HTML(
<style>
#fieldset {
border:none; margin:0; padding:0px; width: 100px; height: 100px;
}
#legend {
padding:0px; margin:0px;
}
</style>
<fieldset id="fieldset">
<legend id="legend" style="margin-top:60px; width:10px; height:20px;"></legend>
<div style="width:20px; height:20px;"></div>
</fieldset>
)HTML");
LayoutUnit kFragmentainerSpaceAvailable(50);
NGBlockNode node(ToLayoutBox(GetLayoutObjectByElementId("fieldset")));
NGConstraintSpace space = ConstructBlockLayoutTestConstraintSpace(
WritingMode::kHorizontalTb, TextDirection::kLtr,
LogicalSize(LayoutUnit(1000), kIndefiniteSize), false,
node.CreatesNewFormattingContext(), kFragmentainerSpaceAvailable);
scoped_refptr<const NGPhysicalBoxFragment> fragment =
NGBaseLayoutAlgorithmTest::RunFieldsetLayoutAlgorithm(node, space);
ASSERT_TRUE(fragment->BreakToken());
String dump = DumpFragmentTree(fragment.get());
String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:100x50
)DUMP";
EXPECT_EQ(expectation, dump);
fragment = NGBaseLayoutAlgorithmTest::RunFieldsetLayoutAlgorithm(
node, space, fragment->BreakToken());
ASSERT_FALSE(fragment->BreakToken());
dump = DumpFragmentTree(fragment.get());
expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:100x50
offset:0,0 size:10x20
offset:0,20 size:100x30
offset:0,0 size:20x20
)DUMP";
EXPECT_EQ(expectation, dump);
}
TEST_F(NGFieldsetLayoutAlgorithmTest, MarginBottomPastEndOfFragmentainer) { TEST_F(NGFieldsetLayoutAlgorithmTest, MarginBottomPastEndOfFragmentainer) {
// A block whose border box would start past the end of the current // A block whose border box would start past the end of the current
// fragmentainer should start exactly at the start of the next fragmentainer, // fragmentainer should start exactly at the start of the next fragmentainer,
...@@ -1982,7 +1930,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, SmallerLegendLargeBorderWithBreak) { ...@@ -1982,7 +1930,7 @@ TEST_F(NGFieldsetLayoutAlgorithmTest, SmallerLegendLargeBorderWithBreak) {
String dump = DumpFragmentTree(fragment.get()); String dump = DumpFragmentTree(fragment.get());
String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::. String expectation = R"DUMP(.:: LayoutNG Physical Fragment Tree ::.
offset:unplaced size:220x60 offset:unplaced size:220x60
offset:60,16 size:10x5 offset:60,27.5 size:10x5
)DUMP"; )DUMP";
EXPECT_EQ(expectation, dump); EXPECT_EQ(expectation, dump);
......
...@@ -1308,10 +1308,6 @@ virtual/layout_ng_fragment_traversal/external/wpt/css/CSS2/text/* [ Skip ] ...@@ -1308,10 +1308,6 @@ virtual/layout_ng_fragment_traversal/external/wpt/css/CSS2/text/* [ Skip ]
# Fieldset in NG # Fieldset in NG
# #
### virtual/layout_ng_fieldset/fast/forms/fieldset/
crbug.com/875235 virtual/layout_ng_fieldset/fast/forms/fieldset/legend-small-after-margin-before-border-horizontal-mode.html [ Failure ]
crbug.com/875235 virtual/layout_ng_fieldset/fast/forms/fieldset/legend-after-margin-with-before-border-horizontal-mode.html [ Failure ]
## Fieldseet in NG - Passing reference tests ## Fieldseet in NG - Passing reference tests
crbug.com/875235 virtual/layout_ng_fieldset/external/wpt/html/rendering/non-replaced-elements/the-fieldset-and-legend-elements/fieldset-vertical.html [ Pass ] crbug.com/875235 virtual/layout_ng_fieldset/external/wpt/html/rendering/non-replaced-elements/the-fieldset-and-legend-elements/fieldset-vertical.html [ Pass ]
......
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