Commit b41bfc20 authored by Morten Stenshorne's avatar Morten Stenshorne Committed by Commit Bot

[LayoutNG] Don't allow breaks at the start of a fragmentainer.

We had this almost right, but failed when there was a break with low
appeal inside a child.

Bug: 829028
Change-Id: I67c0d07db85b8de6d9ac3f685263fb1d3b490760
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1940273Reviewed-by: default avatarKoji Ishii <kojii@chromium.org>
Reviewed-by: default avatarIan Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/master@{#719919}
parent 9f7dc470
......@@ -355,24 +355,33 @@ bool MovePastBreakpoint(const NGConstraintSpace& space,
return true;
}
LayoutUnit space_left =
space.FragmentainerBlockSize() - fragmentainer_block_offset;
// If we haven't used any space at all in the fragmentainer yet, we cannot
// break before this child, or there'd be no progress. We'd risk creating an
// infinite number of fragmentainers without putting any content into them.
bool refuse_break = space_left >= space.FragmentainerBlockSize();
if (IsA<NGBlockBreakToken>(physical_fragment.BreakToken())) {
// The block child broke inside. We now need to decide whether to keep that
// break, or if it would be better to break before it.
NGBreakAppeal appeal_inside = CalculateBreakAppealInside(
space, To<NGBlockNode>(child), layout_result);
// Allow breaking inside if it has the same appeal or higher than breaking
// before or breaking earlier.
if (appeal_inside >= appeal_before &&
(!builder->HasEarlyBreak() ||
appeal_inside >= builder->BreakAppeal())) {
// before or breaking earlier. Also, if breaking before is impossible, break
// inside regardless of appeal, .
if (refuse_break || (appeal_inside >= appeal_before &&
(!builder->HasEarlyBreak() ||
appeal_inside >= builder->BreakAppeal()))) {
builder->SetBreakAppeal(appeal_inside);
return true;
}
} else {
LayoutUnit space_left =
space.FragmentainerBlockSize() - fragmentainer_block_offset;
bool need_break;
if (child.IsMonolithic()) {
if (refuse_break) {
need_break = false;
} else if (child.IsMonolithic()) {
// If the monolithic piece of content (e.g. a line, or block-level
// replaced content) doesn't fit, we need a break.
need_break = fragment.BlockSize() > space_left;
......@@ -385,14 +394,6 @@ bool MovePastBreakpoint(const NGConstraintSpace& space,
need_break = space_left < LayoutUnit() ||
(space_left == LayoutUnit() && fragment.BlockSize());
}
if (need_break) {
// If we haven't used any space at all in the fragmentainer yet, though,
// we cannot break even if we really want to, or there'd be no progress.
// We'd end up creating an infinite number of fragmentainers without
// putting any content into them.
if (space_left >= space.FragmentainerBlockSize())
need_break = false;
}
if (!need_break) {
if (child.IsBlock()) {
......
<!DOCTYPE html>
<link rel="author" title="Morten Stenshorne" href="mailto:mstensho@chromium.org">
<link rel="help" href="https://www.w3.org/TR/css-break-3/#unforced-breaks">
<link rel="match" href="../reference/ref-filled-green-100px-square.xht">
<meta name="assert" content="Cannot break before a child if we're at the block-start of a fragmentainer">
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="columns:2; column-fill:auto; column-gap:0; width:100px; height:100px; background:red;">
<div></div>
<div style="break-inside:avoid; height:200px; background:green;"></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