Skip to content

Commit b6a921e

Browse files
mstenshoCommit Bot
mstensho
authored and
Commit Bot
committed
Revert "Don't skip past auto-height flexboxes in quirks mode."
This reverts commit aa9c26b. Reason for revert: Broke youtube. crbug.com/845431 Original change's description: > Don't skip past auto-height flexboxes in quirks mode. > > Quirky flexbox behavior really shouldn't be necessary, since flexbox was > invented over a decade after quirky browsers were on the street. > > Replace css3/flexbox/percentage-sizes-quirks.html with a copy of > percentage-sizes.html, except that it's in quirks mode. > > The root problem, as far as the DCHECK failure in bug 841276 is > concerned, though, is that LayoutVideo is a very unsuitable container > for pretty much anything (because it's not a LayoutBlock, let alone > LayoutBlockFlow). By default we end up with the following layout tree > for a VIDEO element: > > LayoutVideo VIDEO > LayoutFlexibleBox (relative positioned) DIV class="phase-pre-ready state-no-source" > LayoutBlockFlow DIV > LayoutFlexibleBox DIV > > See the testcase. The VIDEO there is fixed-positioned, and it also has > another fixed-positioned ancestor (regular block otherwise). When > attempting to locate the right ancestor (descendant of VIDEO) in the > testcase to register a percentage height descendant, we don't see the > LayoutVideo object at all on our way up the tree (because it's not > LayoutBlock). So the percentage height descendant is incorrectly > registered with the outer fixed positioned block. This confuses the > layout machinery, so that when re-laying out the outer fixed positioned > block (see the testcase), we mark the inner flexbox for layout, since > it's a percentage height descendant. But we never get down there, since > the containing VIDEO element isn't going to require layout. Thus we'll > fail DCHECKs that require the entire tree to be clean after layout. > > Bug: 841276, 531783 > Change-Id: I5385d1ad9bbe0f9d739418e9d1d7fc630aa3d3a3 > Reviewed-on: https://chromium-review.googlesource.com/1052768 > Commit-Queue: Morten Stenshorne <[email protected]> > Reviewed-by: Christian Biesinger <[email protected]> > Cr-Commit-Position: refs/heads/master@{#560004} [email protected],[email protected] # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 841276, 531783 Change-Id: I53ed4c6d0894267bbd867dcf633eabc580e5d75c Reviewed-on: https://chromium-review.googlesource.com/1068868 Reviewed-by: Morten Stenshorne <[email protected]> Commit-Queue: Morten Stenshorne <[email protected]> Cr-Commit-Position: refs/heads/master@{#560566}
1 parent 46791ae commit b6a921e

File tree

3 files changed

+6
-48
lines changed

3 files changed

+6
-48
lines changed

‎third_party/WebKit/LayoutTests/css3/flexbox/fixedpos-video-in-abspos-quirk-crash.html

Lines changed: 0 additions & 11 deletions
This file was deleted.

‎third_party/WebKit/LayoutTests/css3/flexbox/percentage-sizes-quirks.html

Lines changed: 6 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2,19 +2,17 @@
22
.flexbox {
33
display: flex;
44
}
5-
.column, .column .fixed {
5+
.column {
66
height: 50px;
77
}
8-
.row, .row .fixed {
8+
.row {
99
width: 50px;
1010
}
1111
.container > div {
1212
outline: 2px solid blue;
1313
}
1414
.row > div > div {
1515
height: 20px;
16-
}
17-
.flexbox > div {
1816
flex: 0 0 auto;
1917
}
2018
.column > .flexbox {
@@ -39,6 +37,8 @@
3937
<body onload="checkLayout('.flexbox')">
4038
<div id=log></div>
4139

40+
<div>CompatMode: <script>document.write(document.compatMode)</script></div>
41+
4242
<div class="container row">
4343
<div class="flexbox">
4444
<div style='width: 10px; min-width: 50%;' data-expected-width=25></div>
@@ -48,26 +48,8 @@
4848
</div>
4949
</div>
5050

51-
<div class="container row" style='width: 100px'>
52-
<div class="flexbox fixed">
53-
<div style='width: 10px; min-width: 50%;' data-expected-width=25></div>
54-
<div style='width: 50%;' data-expected-width=25></div>
55-
<div style='width: 10px; max-width: 50%;' data-expected-width=10></div>
56-
<div style='min-width: 10px; width: 100px; max-width: 50%;' data-expected-width=25></div>
57-
</div>
58-
</div>
59-
60-
<div class="container column" style='margin-bottom: 100px'>
61-
<div class="flexbox" style="height: auto">
62-
<div style='height: 10px; min-height: 50%;' data-expected-height=10></div>
63-
<div style='height: 50%;' data-expected-height=0></div>
64-
<div style='height: 10px; max-height: 50%;' data-expected-height=10></div>
65-
<div style='min-height: 10px; height: 100px; max-height: 50%;' data-expected-height=100></div>
66-
</div>
67-
</div>
68-
69-
<div class="container column">
70-
<div class="flexbox fixed">
51+
<div class="container column" style='margin-bottom: 50px'>
52+
<div class="flexbox">
7153
<div style='height: 10px; min-height: 50%;' data-expected-height=25></div>
7254
<div style='height: 50%;' data-expected-height=25></div>
7355
<div style='height: 10px; max-height: 50%;' data-expected-height=10></div>
@@ -83,18 +65,6 @@
8365

8466
<div class="container column">
8567
<div class="flexbox">
86-
<div style="flex: 0 0 50%" data-expected-height=0></div>
87-
</div>
88-
</div>
89-
90-
<div class="container row">
91-
<div class="flexbox fixed">
92-
<div style="flex: 0 0 50%" data-expected-width=25></div>
93-
</div>
94-
</div>
95-
96-
<div class="container column">
97-
<div class="flexbox fixed">
9868
<div style="flex: 0 0 50%" data-expected-height=25></div>
9969
</div>
10070
</div>

‎third_party/blink/renderer/core/layout/layout_box.cc

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3370,7 +3370,6 @@ bool LayoutBox::SkipContainingBlockForPercentHeightCalculation(
33703370
return GetDocument().InQuirksMode() && !containing_block->IsTableCell() &&
33713371
!containing_block->IsOutOfFlowPositioned() &&
33723372
!containing_block->IsLayoutGrid() &&
3373-
!containing_block->IsFlexibleBoxIncludingDeprecated() &&
33743373
containing_block->Style()->LogicalHeight().IsAuto();
33753374
}
33763375

0 commit comments

Comments
 (0)