Commit f199b3c3 authored by Nohemi Fernandez's avatar Nohemi Fernandez Committed by Commit Bot

Revert "[LayoutNG] Don't truncate margins after spanners."

This reverts commit 430b7621.

Reason for revert: Failing builder https://ci.chromium.org/p/chromium/builders/ci/linux-trusty-rel

Original change's description:
> [LayoutNG] Don't truncate margins after spanners.
>
> A column spanner forcefully breaks a column, so any leading margins on
> content right after the spanner shouldn't be discarded.
>
> Bug: 829028
> Change-Id: I767466677d72be2760306ddd531e847f52cfa8ca
> Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2445482
> Reviewed-by: Alison Maher <almaher@microsoft.com>
> Reviewed-by: Ian Kilpatrick <ikilpatrick@chromium.org>
> Commit-Queue: Morten Stenshorne <mstensho@chromium.org>
> Cr-Commit-Position: refs/heads/master@{#813892}

TBR=ikilpatrick@chromium.org,mstensho@chromium.org,almaher@microsoft.com

Change-Id: I535ec21071aacd85f22968286e0f920fd2870324
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Bug: 829028
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2450062Reviewed-by: default avatarNohemi Fernandez <fernandex@chromium.org>
Commit-Queue: Nohemi Fernandez <fernandex@chromium.org>
Cr-Commit-Position: refs/heads/master@{#814142}
parent e76e12a5
...@@ -42,7 +42,6 @@ NGBlockBreakToken::NGBlockBreakToken(PassKey key, ...@@ -42,7 +42,6 @@ NGBlockBreakToken::NGBlockBreakToken(PassKey key,
num_children_(builder.child_break_tokens_.size()) { num_children_(builder.child_break_tokens_.size()) {
break_appeal_ = builder.break_appeal_; break_appeal_ = builder.break_appeal_;
has_seen_all_children_ = builder.has_seen_all_children_; has_seen_all_children_ = builder.has_seen_all_children_;
is_caused_by_column_spanner_ = builder.FoundColumnSpanner();
is_at_block_end_ = builder.is_at_block_end_; is_at_block_end_ = builder.is_at_block_end_;
for (wtf_size_t i = 0; i < builder.child_break_tokens_.size(); ++i) { for (wtf_size_t i = 0; i < builder.child_break_tokens_.size(); ++i) {
child_break_tokens_[i] = builder.child_break_tokens_[i].get(); child_break_tokens_[i] = builder.child_break_tokens_[i].get();
......
...@@ -72,8 +72,6 @@ class CORE_EXPORT NGBlockBreakToken final : public NGBreakToken { ...@@ -72,8 +72,6 @@ class CORE_EXPORT NGBlockBreakToken final : public NGBreakToken {
bool IsForcedBreak() const { return is_forced_break_; } bool IsForcedBreak() const { return is_forced_break_; }
bool IsCausedByColumnSpanner() const { return is_caused_by_column_spanner_; }
// Return true if all children have been "seen". When we have reached this // Return true if all children have been "seen". When we have reached this
// point, and resume layout in a fragmentainer, we should only process child // point, and resume layout in a fragmentainer, we should only process child
// break tokens, if any, and not attempt to start laying out nodes that don't // break tokens, if any, and not attempt to start laying out nodes that don't
......
...@@ -2489,7 +2489,7 @@ NGConstraintSpace NGBlockLayoutAlgorithm::CreateConstraintSpaceForChild( ...@@ -2489,7 +2489,7 @@ NGConstraintSpace NGBlockLayoutAlgorithm::CreateConstraintSpaceForChild(
container_builder_.AdjoiningObjectTypes()); container_builder_.AdjoiningObjectTypes());
} }
builder.SetLinesUntilClamp(lines_until_clamp_); builder.SetLinesUntilClamp(lines_until_clamp_);
} else if (child_data.allow_discard_start_margin) { } else if (child_data.is_resuming_after_break) {
// If the child is being resumed after a break, margins inside the child may // If the child is being resumed after a break, margins inside the child may
// be adjoining with the fragmentainer boundary, regardless of whether the // be adjoining with the fragmentainer boundary, regardless of whether the
// child establishes a new formatting context or not. // child establishes a new formatting context or not.
......
...@@ -45,7 +45,7 @@ struct NGInflowChildData { ...@@ -45,7 +45,7 @@ struct NGInflowChildData {
NGMarginStrut margin_strut; NGMarginStrut margin_strut;
NGBoxStrut margins; NGBoxStrut margins;
bool margins_fully_resolved; bool margins_fully_resolved;
bool allow_discard_start_margin; bool is_resuming_after_break;
}; };
// A class for general block layout (e.g. a <div> with no special style). // A class for general block layout (e.g. a <div> with no special style).
......
...@@ -78,7 +78,6 @@ class CORE_EXPORT NGBreakToken : public RefCounted<NGBreakToken> { ...@@ -78,7 +78,6 @@ class CORE_EXPORT NGBreakToken : public RefCounted<NGBreakToken> {
flags_(0), flags_(0),
is_break_before_(false), is_break_before_(false),
is_forced_break_(false), is_forced_break_(false),
is_caused_by_column_spanner_(false),
is_at_block_end_(false), is_at_block_end_(false),
break_appeal_(kBreakAppealPerfect), break_appeal_(kBreakAppealPerfect),
has_seen_all_children_(false) { has_seen_all_children_(false) {
...@@ -106,8 +105,6 @@ class CORE_EXPORT NGBreakToken : public RefCounted<NGBreakToken> { ...@@ -106,8 +105,6 @@ class CORE_EXPORT NGBreakToken : public RefCounted<NGBreakToken> {
unsigned is_forced_break_ : 1; unsigned is_forced_break_ : 1;
unsigned is_caused_by_column_spanner_ : 1;
// Set when layout is past the block-end border edge. If we break when we're // Set when layout is past the block-end border edge. If we break when we're
// in this state, it means that something is overflowing, and thus establishes // in this state, it means that something is overflowing, and thus establishes
// a parallel flow. // a parallel flow.
......
...@@ -561,8 +561,9 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow( ...@@ -561,8 +561,9 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow(
scoped_refptr<const NGBlockBreakToken> column_break_token = scoped_refptr<const NGBlockBreakToken> column_break_token =
next_column_token; next_column_token;
bool allow_discard_start_margin = // This is the first column in this fragmentation context if there are no
column_break_token && !column_break_token->IsCausedByColumnSpanner(); // preceding columns in this row and there are also no preceding rows.
bool is_first_fragmentainer = !column_break_token && !BreakToken();
LayoutUnit column_inline_offset(BorderScrollbarPadding().inline_start); LayoutUnit column_inline_offset(BorderScrollbarPadding().inline_start);
int actual_column_count = 0; int actual_column_count = 0;
...@@ -578,7 +579,7 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow( ...@@ -578,7 +579,7 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow(
// Lay out one column. Each column will become a fragment. // Lay out one column. Each column will become a fragment.
NGConstraintSpace child_space = CreateConstraintSpaceForColumns( NGConstraintSpace child_space = CreateConstraintSpaceForColumns(
ConstraintSpace(), column_size, ColumnPercentageResolutionSize(), ConstraintSpace(), column_size, ColumnPercentageResolutionSize(),
allow_discard_start_margin, balance_columns); is_first_fragmentainer, balance_columns);
NGFragmentGeometry fragment_geometry = NGFragmentGeometry fragment_geometry =
CalculateInitialFragmentGeometry(child_space, Node()); CalculateInitialFragmentGeometry(child_space, Node());
...@@ -631,7 +632,7 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow( ...@@ -631,7 +632,7 @@ scoped_refptr<const NGLayoutResult> NGColumnLayoutAlgorithm::LayoutRow(
break; break;
} }
allow_discard_start_margin = true; is_first_fragmentainer = false;
} while (column_break_token); } while (column_break_token);
// TODO(mstensho): Nested column balancing. // TODO(mstensho): Nested column balancing.
......
...@@ -735,7 +735,7 @@ NGConstraintSpace CreateConstraintSpaceForColumns( ...@@ -735,7 +735,7 @@ NGConstraintSpace CreateConstraintSpaceForColumns(
const NGConstraintSpace& parent_space, const NGConstraintSpace& parent_space,
LogicalSize column_size, LogicalSize column_size,
LogicalSize percentage_resolution_size, LogicalSize percentage_resolution_size,
bool allow_discard_start_margin, bool is_first_fragmentainer,
bool balance_columns) { bool balance_columns) {
NGConstraintSpaceBuilder space_builder( NGConstraintSpaceBuilder space_builder(
parent_space, parent_space.GetWritingMode(), /* is_new_fc */ true); parent_space, parent_space.GetWritingMode(), /* is_new_fc */ true);
...@@ -754,12 +754,11 @@ NGConstraintSpace CreateConstraintSpaceForColumns( ...@@ -754,12 +754,11 @@ NGConstraintSpace CreateConstraintSpaceForColumns(
space_builder.SetIsInColumnBfc(); space_builder.SetIsInColumnBfc();
if (balance_columns) if (balance_columns)
space_builder.SetIsInsideBalancedColumns(); space_builder.SetIsInsideBalancedColumns();
if (allow_discard_start_margin) { if (!is_first_fragmentainer) {
// Unless it's the first column in the multicol container, or the first // Margins at fragmentainer boundaries should be eaten and truncated to
// column after a spanner, margins at fragmentainer boundaries should be // zero. Note that this doesn't apply to margins at forced breaks, but we'll
// eaten and truncated to zero. Note that this doesn't apply to margins at // deal with those when we get to them. Set up a margin strut that eats all
// forced breaks, but we'll deal with those when we get to them. Set up a // leading adjacent margins.
// margin strut that eats all leading adjacent margins.
space_builder.SetDiscardingMarginStrut(); space_builder.SetDiscardingMarginStrut();
} }
......
...@@ -264,7 +264,7 @@ NGConstraintSpace CreateConstraintSpaceForColumns( ...@@ -264,7 +264,7 @@ NGConstraintSpace CreateConstraintSpaceForColumns(
const NGConstraintSpace& parent_space, const NGConstraintSpace& parent_space,
LogicalSize column_size, LogicalSize column_size,
LogicalSize percentage_resolution_size, LogicalSize percentage_resolution_size,
bool allow_discard_start_margin, bool is_first_fragmentainer,
bool balance_columns); bool balance_columns);
// Return the adjusted child margin to be applied at the end of a fragment. // Return the adjusted child margin to be applied at the end of a fragment.
......
...@@ -1172,16 +1172,7 @@ const NGConstraintSpace& NGOutOfFlowLayoutPart::GetFragmentainerConstraintSpace( ...@@ -1172,16 +1172,7 @@ const NGConstraintSpace& NGOutOfFlowLayoutPart::GetFragmentainerConstraintSpace(
wtf_size_t num_children = container_builder_->Children().size(); wtf_size_t num_children = container_builder_->Children().size();
bool is_new_fragment = index >= num_children; bool is_new_fragment = index >= num_children;
// Allow margins to be discarded if this is not the first column in the bool is_first_fragmentainer = index == 0;
// multicol container, and we're not right after a spanner.
//
// TODO(layout-dev): This check is incorrect in nested multicol. If the
// previous outer fragmentainer ended with regular column content (i.e. not a
// spanner), and this is the first column in the next outer fragmentainer, we
// should still discard margins, since there is no explicit break involved.
bool allow_discard_start_margin =
is_new_fragment || (index > 0 && container_builder_->Children()[index - 1]
.fragment->IsFragmentainerBox());
// If we are a new fragment, find a non-spanner fragmentainer to base our // If we are a new fragment, find a non-spanner fragmentainer to base our
// constraint space off of. // constraint space off of.
...@@ -1222,7 +1213,7 @@ const NGConstraintSpace& NGOutOfFlowLayoutPart::GetFragmentainerConstraintSpace( ...@@ -1222,7 +1213,7 @@ const NGConstraintSpace& NGOutOfFlowLayoutPart::GetFragmentainerConstraintSpace(
NGConstraintSpace fragmentainer_constraint_space = NGConstraintSpace fragmentainer_constraint_space =
CreateConstraintSpaceForColumns(*container_builder_->ConstraintSpace(), CreateConstraintSpaceForColumns(*container_builder_->ConstraintSpace(),
column_size, percentage_resolution_size, column_size, percentage_resolution_size,
allow_discard_start_margin, is_first_fragmentainer,
/* balance_columns */ false); /* balance_columns */ false);
return fragmentainer_constraint_space_map_ return fragmentainer_constraint_space_map_
......
...@@ -979,7 +979,6 @@ virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-006 ...@@ -979,7 +979,6 @@ virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-006
virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-007.html [ Pass ] virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-007.html [ Pass ]
virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-008.html [ Pass ] virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-008.html [ Pass ]
virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-014.html [ Pass ] virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-014.html [ Pass ]
virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-015.html [ Pass ]
virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-fieldset-001.html [ Pass ] virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-fieldset-001.html [ Pass ]
virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-margin-bottom-001.xht [ Pass ] virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-all-margin-bottom-001.xht [ Pass ]
virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-float-001.xht [ Pass ] virtual/layout_ng_block_frag/external/wpt/css/css-multicol/multicol-span-float-001.xht [ Pass ]
...@@ -3275,7 +3274,6 @@ crbug.com/915204 external/wpt/css/css-multicol/multicol-span-all-008.html [ Fail ...@@ -3275,7 +3274,6 @@ crbug.com/915204 external/wpt/css/css-multicol/multicol-span-all-008.html [ Fail
crbug.com/926685 external/wpt/css/css-multicol/multicol-span-all-010.html [ Failure ] crbug.com/926685 external/wpt/css/css-multicol/multicol-span-all-010.html [ Failure ]
crbug.com/915204 external/wpt/css/css-multicol/multicol-span-all-011.html [ Failure ] crbug.com/915204 external/wpt/css/css-multicol/multicol-span-all-011.html [ Failure ]
crbug.com/829028 external/wpt/css/css-multicol/multicol-span-all-014.html [ Failure ] crbug.com/829028 external/wpt/css/css-multicol/multicol-span-all-014.html [ Failure ]
crbug.com/829028 external/wpt/css/css-multicol/multicol-span-all-015.html [ Failure ]
crbug.com/892817 external/wpt/css/css-multicol/multicol-span-all-margin-bottom-001.xht [ Failure ] crbug.com/892817 external/wpt/css/css-multicol/multicol-span-all-margin-bottom-001.xht [ Failure ]
crbug.com/636055 external/wpt/css/css-multicol/multicol-span-all-margin-nested-002.xht [ Failure ] crbug.com/636055 external/wpt/css/css-multicol/multicol-span-all-margin-nested-002.xht [ Failure ]
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 ]
......
<!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/#spanning-columns">
<link rel="help" href="https://www.w3.org/TR/css-break-3/#break-margins">
<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; column-gap:0; width:100px; background:red;">
<div style="height:30px;"></div>
<div style="height:30px; background:green;"></div>
<div>
<div style="column-span:all; height:40px; background:green;">
<div style="width:50px; height:40px; background:red;"></div>
</div>
</div>
<div style="margin-top:-70px; height:130px; 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