Skip to content

Commit

Permalink
Only add trailing border/padding to the right fragment.
Browse files Browse the repository at this point in the history
We used to always subtract trailing border/padding for every fragment
that shouldn't have it, but this changed for table cells in CL:4303193.
To fix this, make sure that we only include trailing border/padding when
at the fragment that has the block-end edge of the box.

Bug: 1425075
Change-Id: Id7afa2eee764e1a8bb3017a90b212495278fe623
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4345873
Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1118245}
  • Loading branch information
mstensho authored and Chromium LUCI CQ committed Mar 16, 2023
1 parent b45b343 commit 25fc0f6
Show file tree
Hide file tree
Showing 3 changed files with 33 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -876,14 +876,16 @@ const NGLayoutResult* NGBlockLayoutAlgorithm::FinishLayout(
ExclusionSpace().ClearanceOffsetIncludingInitialLetter(EClear::kBoth));
}

LayoutUnit block_end_border_padding = BorderScrollbarPadding().block_end;

// If line clamping occurred, the intrinsic block-size comes from the
// intrinsic block-size at the time of the clamp.
if (intrinsic_block_size_when_clamped_) {
DCHECK(container_builder_.BfcBlockOffset());
intrinsic_block_size_ = *intrinsic_block_size_when_clamped_ +
BorderScrollbarPadding().block_end;
intrinsic_block_size_ =
*intrinsic_block_size_when_clamped_ + block_end_border_padding;
end_margin_strut = NGMarginStrut();
} else if (BorderScrollbarPadding().block_end ||
} else if (block_end_border_padding ||
previous_inflow_position->self_collapsing_child_had_clearance ||
ConstraintSpace().IsNewFormattingContext()) {
// The end margin strut of an in-flow fragment contributes to the size of
Expand Down Expand Up @@ -938,7 +940,15 @@ const NGLayoutResult* NGBlockLayoutAlgorithm::FinishLayout(
previous_inflow_position->logical_block_offset + margin_strut_sum);
}

intrinsic_block_size_ += BorderScrollbarPadding().block_end;
if ((BreakToken() && BreakToken()->IsAtBlockEnd()) ||
(container_builder_.HasInflowChildBreakInside() &&
!container_builder_.IsKnownToFitInFragmentainer())) {
// The block-end edge isn't in this fragment. We either haven't got there
// yet, or we're past it (and are overflowing). So don't add trailing
// border/padding.
block_end_border_padding = LayoutUnit();
}
intrinsic_block_size_ += block_end_border_padding;
end_margin_strut = NGMarginStrut();
} else {
// Update our intrinsic block size to be just past the block-end border edge
Expand Down Expand Up @@ -1040,7 +1050,7 @@ const NGLayoutResult* NGBlockLayoutAlgorithm::FinishLayout(
}

if (UNLIKELY(InvolvedInBlockFragmentation(container_builder_))) {
NGBreakStatus status = FinalizeForFragmentation();
NGBreakStatus status = FinalizeForFragmentation(block_end_border_padding);
if (status != NGBreakStatus::kContinue) {
if (status == NGBreakStatus::kNeedsEarlierBreak)
return container_builder_.Abort(NGLayoutResult::kNeedsEarlierBreak);
Expand Down Expand Up @@ -2386,7 +2396,8 @@ void NGBlockLayoutAlgorithm::ConsumeRemainingFragmentainerSpace(
}
}

NGBreakStatus NGBlockLayoutAlgorithm::FinalizeForFragmentation() {
NGBreakStatus NGBlockLayoutAlgorithm::FinalizeForFragmentation(
LayoutUnit block_end_border_padding_added) {
if (Node().IsInlineFormattingContextRoot() && !early_break_ &&
ConstraintSpace().HasBlockFragmentation()) {
if (container_builder_.HasInflowChildBreakInside() ||
Expand Down Expand Up @@ -2428,7 +2439,7 @@ NGBreakStatus NGBlockLayoutAlgorithm::FinalizeForFragmentation() {
space_left = FragmentainerSpaceLeft(ConstraintSpace());

return FinishFragmentation(Node(), ConstraintSpace(),
BorderPadding().block_end, space_left,
block_end_border_padding_added, space_left,
&container_builder_);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,8 @@ class CORE_EXPORT NGBlockLayoutAlgorithm
// clipped box gets overflowed past the fragmentation line). The return value
// can be checked for this. Only if kContinue is returned, can a fragment be
// created.
NGBreakStatus FinalizeForFragmentation();
NGBreakStatus FinalizeForFragmentation(
LayoutUnit block_end_border_padding_added);

// Insert a fragmentainer break before the child if necessary.
// See |::blink::BreakBeforeChildIfNeeded()| for more documentation.
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<!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/#box-splitting">
<link rel="help" href="https://bugs.chromium.org/p/chromium/issues/detail?id=1425075">
<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:2; gap:0; column-fill:auto; width:100px; height:100px; background:red;">
<div style="display:table; width:100%;">
<div style="display:table-cell; border-bottom:50px solid green; background:red;">
<div style="height:150px; background:green;"></div>
</div>
</div>
</div>

0 comments on commit 25fc0f6

Please sign in to comment.