Commit 3f74dd89 authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot

[LayoutNG] Set column fragments to the actual column size.

Just letting the fragmentation machinery do what it would do if it gets
to treat column boxes as regular column content was no conscious choice,
and it turns out that setting the column fragments to the actual column
size is better, or we get some trouble with overflow, both when it comes
to painting column rules, and the fact that we get spurious empty column
fragments after unbreakable content (new unit test
NGColumnLayoutAlgorithmTest.TallReplacedContent).

Remove fast/multicol/rule-in-nested-with-too-tall-line.html and replace
it with a valid test (that still fails both in legacy and NG; in NG
because we fail to paint text on a general basis).

Bug: 829028
Change-Id: I3656c299c7a87bea29e1790477cf524b9522fa97
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1948923Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#721376}
parent 604ea7b2
...@@ -2078,29 +2078,38 @@ bool NGBlockLayoutAlgorithm::FinalizeForFragmentation() { ...@@ -2078,29 +2078,38 @@ bool NGBlockLayoutAlgorithm::FinalizeForFragmentation() {
LayoutUnit consumed_block_size = LayoutUnit consumed_block_size =
BreakToken() ? BreakToken()->ConsumedBlockSize() : LayoutUnit(); BreakToken() ? BreakToken()->ConsumedBlockSize() : LayoutUnit();
LayoutUnit block_size =
ComputeBlockSizeForFragment(ConstraintSpace(), Style(), border_padding_,
consumed_block_size + intrinsic_block_size_);
block_size -= consumed_block_size;
DCHECK_GE(block_size, LayoutUnit())
<< "Adding and subtracting the consumed_block_size shouldn't leave the "
"block_size for this fragment smaller than zero.";
LayoutUnit space_left = FragmentainerSpaceAvailable(); LayoutUnit space_left = FragmentainerSpaceAvailable();
LayoutUnit block_size;
if (space_left <= LayoutUnit()) { if (container_builder_.BoxType() == NGPhysicalFragment::kColumnBox &&
// The amount of space available may be zero, or even negative, if the ConstraintSpace().HasKnownFragmentainerBlockSize()) {
// border-start edge of this block starts exactly at, or even after the // We're building column fragments, and we know the column size. Just use
// fragmentainer boundary. We're going to need a break before this block, // that. Calculating the size the regular way would cause some problems with
// because no part of it fits in the current fragmentainer. Due to margin // overflow. For one, we don't want to produce a break token if there's no
// collapsing with children, this situation is something that we cannot // child content that requires it.
// always detect prior to layout. The fragment produced by this algorithm is block_size = ConstraintSpace().FragmentainerBlockSize();
// going to be thrown away. The parent layout algorithm will eventually } else {
// detect that there's no room for a fragment for this node, and drop the block_size = ComputeBlockSizeForFragment(
// fragment on the floor. Therefore it doesn't matter how we set up the ConstraintSpace(), Style(), border_padding_,
// container builder, so just return. consumed_block_size + intrinsic_block_size_);
return true;
block_size -= consumed_block_size;
DCHECK_GE(block_size, LayoutUnit())
<< "Adding and subtracting the consumed_block_size shouldn't leave the "
"block_size for this fragment smaller than zero.";
if (space_left <= LayoutUnit()) {
// The amount of space available may be zero, or even negative, if the
// border-start edge of this block starts exactly at, or even after the
// fragmentainer boundary. We're going to need a break before this block,
// because no part of it fits in the current fragmentainer. Due to margin
// collapsing with children, this situation is something that we cannot
// always detect prior to layout. The fragment produced by this algorithm
// is going to be thrown away. The parent layout algorithm will eventually
// detect that there's no room for a fragment for this node, and drop the
// fragment on the floor. Therefore it doesn't matter how we set up the
// container builder, so just return.
return true;
}
} }
FinishFragmentation(ConstraintSpace(), block_size, intrinsic_block_size_, FinishFragmentation(ConstraintSpace(), block_size, intrinsic_block_size_,
......
...@@ -885,8 +885,6 @@ void NGBoxFragmentPainter::PaintColumnRules( ...@@ -885,8 +885,6 @@ void NGBoxFragmentPainter::PaintColumnRules(
center = (current_column.X() + previous_column.Right()) / 2; center = (current_column.X() + previous_column.Right()) / 2;
box_side = BoxSide::kRight; box_side = BoxSide::kRight;
} }
// The last column may be shorter than the previous ones, but otherwise
// they should be the same.
LayoutUnit rule_length = previous_column.Height(); LayoutUnit rule_length = previous_column.Height();
DCHECK_GE(rule_length, current_column.Height()); DCHECK_GE(rule_length, current_column.Height());
rule.offset.top = previous_column.offset.top; rule.offset.top = previous_column.offset.top;
...@@ -905,8 +903,6 @@ void NGBoxFragmentPainter::PaintColumnRules( ...@@ -905,8 +903,6 @@ void NGBoxFragmentPainter::PaintColumnRules(
center = (current_column.Y() + previous_column.Bottom()) / 2; center = (current_column.Y() + previous_column.Bottom()) / 2;
box_side = BoxSide::kBottom; box_side = BoxSide::kBottom;
} }
// The last column may be shorter than the previous ones, but otherwise
// they should be the same.
LayoutUnit rule_length = previous_column.Width(); LayoutUnit rule_length = previous_column.Width();
DCHECK_GE(rule_length, current_column.Width()); DCHECK_GE(rule_length, current_column.Width());
rule.offset.left = previous_column.offset.left; rule.offset.left = previous_column.offset.left;
......
...@@ -1459,7 +1459,6 @@ crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/pushed-line-affected ...@@ -1459,7 +1459,6 @@ crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/pushed-line-affected
crbug.com/797591 virtual/layout_ng_block_frag/fast/multicol/regular-block-becomes-multicol.html [ Pass ] crbug.com/797591 virtual/layout_ng_block_frag/fast/multicol/regular-block-becomes-multicol.html [ Pass ]
crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/relayout-and-push-float.html [ Failure ] crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/relayout-and-push-float.html [ Failure ]
crbug.com/982194 virtual/layout_ng_block_frag/fast/multicol/relpos-inside-inline-block.html [ Failure ] crbug.com/982194 virtual/layout_ng_block_frag/fast/multicol/relpos-inside-inline-block.html [ Failure ]
crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/rule-in-nested-with-too-tall-line.html [ Failure ]
crbug.com/714962 virtual/layout_ng_block_frag/fast/multicol/scale-transform-text.html [ Failure ] crbug.com/714962 virtual/layout_ng_block_frag/fast/multicol/scale-transform-text.html [ Failure ]
crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/shadow-breaking.html [ Failure ] crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/shadow-breaking.html [ Failure ]
crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/span/as-inner-multicol.html [ Failure ] crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/span/as-inner-multicol.html [ Failure ]
......
...@@ -1030,6 +1030,7 @@ crbug.com/714962 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/mult ...@@ -1030,6 +1030,7 @@ crbug.com/714962 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/mult
crbug.com/714962 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-width-large-002.xht [ Failure ] crbug.com/714962 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-width-large-002.xht [ Failure ]
crbug.com/591099 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-width-small-001.xht [ Failure ] crbug.com/591099 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-width-small-001.xht [ Failure ]
crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-zero-height-001.xht [ Failure ] crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-zero-height-001.xht [ Failure ]
crbug.com/829028 virtual/layout_ng_block_frag/external/wpt/css/css-multicol/nested-with-too-tall-line.html [ Failure ]
crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/abspos-after-break-after.html [ Failure ] crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/abspos-after-break-after.html [ Failure ]
crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/abspos-new-width-rebalance.html [ Crash Failure ] crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/abspos-new-width-rebalance.html [ Crash Failure ]
crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/balance-float-after-forced-break.html [ Failure ] crbug.com/591099 virtual/layout_ng_block_frag/fast/multicol/balance-float-after-forced-break.html [ Failure ]
...@@ -3979,6 +3980,7 @@ crbug.com/636055 external/wpt/css/css-multicol/multicol-span-all-margin-nested-0 ...@@ -3979,6 +3980,7 @@ crbug.com/636055 external/wpt/css/css-multicol/multicol-span-all-margin-nested-0
crbug.com/990240 external/wpt/css/css-multicol/multicol-span-all-rule-001.html [ Failure ] crbug.com/990240 external/wpt/css/css-multicol/multicol-span-all-rule-001.html [ Failure ]
crbug.com/792446 external/wpt/css/css-multicol/multicol-span-float-001.xht [ Failure ] crbug.com/792446 external/wpt/css/css-multicol/multicol-span-float-001.xht [ Failure ]
crbug.com/964183 external/wpt/css/css-multicol/multicol-width-005.html [ Failure ] crbug.com/964183 external/wpt/css/css-multicol/multicol-width-005.html [ Failure ]
crbug.com/829028 external/wpt/css/css-multicol/nested-with-too-tall-line.html [ Failure ]
crbug.com/796961 external/wpt/css/css-pseudo/marker-content-002.html [ Failure ] crbug.com/796961 external/wpt/css/css-pseudo/marker-content-002.html [ Failure ]
crbug.com/796961 external/wpt/css/css-pseudo/marker-content-005.html [ Failure ] crbug.com/796961 external/wpt/css/css-pseudo/marker-content-005.html [ Failure ]
......
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
<link rel="help" href="https://www.w3.org/TR/css-multicol-1/#column-gaps-and-rules">
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="columns:4; column-fill:auto; column-gap:5px; column-rule:5px solid green; width:100px; height:100px; background:red;">
<div style="height:50px;">
<div style="height:400px; background:green;"></div>
</div>
</div>
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
<p>The word "PASS" should be seen below, and there should be no red.</p>
<div style="line-height:200px;">
PASS
</div>
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
<link rel="help" href="https://www.w3.org/TR/css-multicol-1/#the-multi-column-model">
<link rel="help" href="https://drafts.csswg.org/css-break/#unforced-breaks">
<link rel="match" href="nested-with-too-tall-line-ref.html">
<style>
.multicol { columns:2; column-fill:auto; }
</style>
<p>The word "PASS" should be seen below, and there should be no red.</p>
<div class="multicol" style="width:100px; height:100px; line-height:200px;">
<div class="multicol" style="height:200px; column-rule:solid red;">
PASS
</div>
</div>
<!DOCTYPE html>
<p>There should only be <em>one</em> vertical line below.</p>
<div style="margin-left:23px; border-left:4px solid; width:0; height:100px;"></div>
<!DOCTYPE html>
<style>
.multicol { columns:2; column-gap:0; column-fill:auto; }
</style>
<p>There should only be <em>one</em> vertical line below.</p>
<div class="multicol" style="width:100px; height:100px; line-height:200px;">
<div class="multicol" style="column-rule:4px solid; height:200px;">
<br>
</div>
</div>
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