Skip to content

Commit

Permalink
[FlexNG] Multi-line flex columns and margins
Browse files Browse the repository at this point in the history 
Add tests for margin handling and multi-line column flex
containers. No extra code was needed to get this working. However,
while adding tests, a bug was discovered in the iterator. If we
call NextLine() while processing child break tokens, we shouldn't
set next_unstarted_item_ as that will cause a DCHECK failure.

Bug: 660611
Change-Id: I3fdcd558492d4c76de8c24777c0776ee3fd93ec2
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3537336
Reviewed-by: Morten Stenshorne <mstensho@chromium.org>
Commit-Queue: Alison Maher <almaher@microsoft.com>
Cr-Commit-Position: refs/heads/main@{#983446}
  • Loading branch information
@alisonmaher
alisonmaher authored and Chromium LUCI CQ committed Mar 21, 2022
1 parent 7e2ee66 commit eb57ea3
Show file tree
Hide file tree
Showing 6 changed files with 171 additions and 2 deletions.
6 changes: 4 additions & 2 deletions third_party/blink/renderer/core/layout/ng/flex/ng_flex_item_iterator.cc
View file
Expand Up @@ -85,6 +85,7 @@ NGFlexItemIterator::Entry NGFlexItemIterator::NextItem(bool broke_before_row) {
//
// Note: Rows don't produce a layout result, so if the row broke
// before, the first item in the row will have a broken before.
break_token_ = nullptr;
NextLine();
} else if (!break_token_->HasSeenAllChildren()) {
if (!is_horizontal_flow_) {
Expand All @@ -93,8 +94,8 @@ NGFlexItemIterator::Entry NGFlexItemIterator::NextItem(bool broke_before_row) {
flex_item_idx_ = next_item_idx_for_line_[flex_line_idx_];
}
next_unstarted_item_ = FindNextItem();
break_token_ = nullptr;
}
break_token_ = nullptr;
}
}
} else {
Expand Down Expand Up @@ -150,7 +151,8 @@ void NGFlexItemIterator::NextLine() {
return;
flex_line_idx_++;
AdjustItemIndexForNewLine();
next_unstarted_item_ = FindNextItem();
if (!break_token_)
next_unstarted_item_ = FindNextItem();
}

void NGFlexItemIterator::AdjustItemIndexForNewLine() {
Expand Down
6 changes: 6 additions & 0 deletions third_party/blink/web_tests/TestExpectations
View file
Expand Up @@ -1663,6 +1663,9 @@ virtual/layout_ng_flex_frag/external/wpt/css/css-break/flexbox/multi-line-column
virtual/layout_ng_flex_frag/external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-034.html [ Pass ]
virtual/layout_ng_flex_frag/external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-035.html [ Pass ]
virtual/layout_ng_flex_frag/external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-036.tentative.html [ Pass ]
virtual/layout_ng_flex_frag/external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-037.html [ Pass ]
virtual/layout_ng_flex_frag/external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-039.html [ Pass ]
virtual/layout_ng_flex_frag/external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-040.html [ Pass ]
virtual/layout_ng_flex_frag/external/wpt/css/css-break/flexbox/multi-line-row-flex-fragmentation-007.html [ Pass ]
virtual/layout_ng_flex_frag/external/wpt/css/css-break/flexbox/multi-line-row-flex-fragmentation-008.html [ Pass ]
virtual/layout_ng_flex_frag/external/wpt/css/css-break/flexbox/multi-line-row-flex-fragmentation-010.html [ Pass ]
Expand Down Expand Up @@ -4417,6 +4420,9 @@ crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-column-flex-fragm
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-034.html [ Failure ]
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-035.html [ Failure ]
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-036.tentative.html [ Failure ]
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-037.html [ Failure ]
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-039.html [ Failure ]
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-040.html [ Failure ]
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-row-flex-fragmentation-007.html [ Failure ]
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-row-flex-fragmentation-008.html [ Failure ]
crbug.com/660611 external/wpt/css/css-break/flexbox/multi-line-row-flex-fragmentation-010.html [ Failure ]
Expand Down
46 changes: 46 additions & 0 deletions ...eb_tests/external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-037.html
View file
@@ -0,0 +1,46 @@
<!DOCTYPE html>
<title>
Tests that flex-items get pushed down due to a previous flex row expanding as
a result of fragmentation with margin-top.
</title>
<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#pagination">
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<style>
#flex {
display: flex;
flex-direction: column;
flex-wrap: wrap;
height: 500px;
}
#flex > div {
background: green;
width: 10px;
}
</style>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="width: 100px; height: 100px; columns: 5; column-gap: 0; column-fill: auto; background: red;">
<div id="flex">
<div>
<div style="contain: size; width: 10px; height: 70px;"></div>
<div style="contain: size; width: 10px; height: 40px;"></div>
</div>
<div style="margin-top: 10px; position: relative;">
<div style="contain: size; width: 10px; height: 80px;"></div>
<div style="contain: size; width: 10px; height: 40px;"></div>
<div style="position: absolute; top: -60px; width: 10px; height: 60px; background: green;"></div>
</div>
<div style="height: 100px;"></div>
<div style="height: 60px;"></div>
<div>
<div style="contain: size; width: 10px; height: 70px;"></div>
<div style="contain: size; width: 10px; height: 40px;"></div>
</div>
<div style="margin-top: 60px; position: relative;">
<div style="contain: size; width: 10px; height: 80px;"></div>
<div style="contain: size; width: 10px; height: 40px;"></div>
<div style="position: absolute; top: -60px; width: 10px; height: 60px; background: green;"></div>
</div>
<div style="height: 100px;"></div>
<div style="height: 60px;"></div>
</div>
</div>
31 changes: 31 additions & 0 deletions ...eb_tests/external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-038.html
View file
@@ -0,0 +1,31 @@
<!DOCTYPE html>
<title>
Multi-line column flex fragmentation: Tall margins.
</title>
<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#pagination">
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<style>
#flex {
display: flex;
flex-direction: column;
flex-wrap: wrap;
height: 500px;
position: relative;
}
#flex > div {
background: green;
width: 10px;
}
</style>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="width: 100px; height: 100px; columns: 5; column-gap: 0; column-fill: auto; background: red;">
<div style="height: 50px; width: 20px; background: green;"></div>
<div id="flex">
<div style="height: 100px; margin-top: 50px;"></div>
<div style="height: 150px; margin-top: 150px;"></div>
<div style="height: 100px; margin-top: 50px;"></div>
<div style="height: 150px; margin-top: 150px;"></div>
<div style="position: absolute; height: 50px; width: 20px;"></div>
<div style="position: absolute; height: 150px; width: 20px; top: 150px;"></div>
</div>
</div>
33 changes: 33 additions & 0 deletions ...eb_tests/external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-039.html
View file
@@ -0,0 +1,33 @@
<!DOCTYPE html>
<title>
Multi-line column flex fragmentation: Tall margins with forced break.
</title>
<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#pagination">
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<style>
#flex {
display: flex;
flex-direction: column;
flex-wrap: wrap;
height: 500px;
position: relative;
}
#flex > div {
background: green;
width: 10px;
}
</style>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="width: 100px; height: 100px; columns: 5; column-gap: 0; column-fill: auto; background: red;">
<div id="flex">
<div style="height: 50px;"></div>
<div style="height: 50px; margin-top: 100px; break-before: column;"></div>
<div style="height: 200px; margin-top: 50px;"></div>
<div style="height: 50px; background: white;"></div>
<div style="height: 50px; margin-top: 100px; break-before: column;"></div>
<div style="height: 200px; margin-top: 50px;"></div>
<div style="position: absolute; height: 150px; top: 50px;"></div>
<div style="position: absolute; height: 200px; left: 10px;"></div>
<div style="position: absolute; height: 50px; width: 20px; top: 250px;"></div>
</div>
</div>
51 changes: 51 additions & 0 deletions ...eb_tests/external/wpt/css/css-break/flexbox/multi-line-column-flex-fragmentation-040.html
View file
@@ -0,0 +1,51 @@
<!DOCTYPE html>
<title>
Multi-line column flex fragmentation: Negative margins.
</title>
<link rel="help" href="https://drafts.csswg.org/css-flexbox-1/#pagination">
<link rel="match" href="../../reference/ref-filled-green-100px-square.xht">
<style>
#flex {
display: flex;
flex-direction: column;
flex-wrap: wrap;
height: 500px;
}
#flex > div {
height: 100px;
width: 10px;
}
#flex > div:nth-child(odd) {
background: green;
margin-top: -100px;
break-before: column;
}
</style>
<p>Test passes if there is a filled green square and <strong>no red</strong>.</p>
<div style="height: 100px; width: 100px; background: red; position: absolute">
<div style="position: absolute; left: 10px; width: 10px; height: 100px; background: green;"></div>
<div style="margin-top: 100px; width: 100px; height: 100px; columns: 5; column-gap: 0; column-fill: auto;">
<div id="flex">
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div style="background: white;"></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
<div></div>
</div>
</div>
</div>

0 comments on commit eb57ea3

Please sign in to comment.