Skip to content

Commit faa9a85

Browse files
mstenshochromium-wpt-export-bot
authored andcommitted
Don't include BR clearance in the line's height.
Including clearance in the line height may result in tall monolithic content, which is problematic for block fragmentation, especially for column balancing. Instead, store trailing clearance in the layout result, so that the block layout algorithm can add it to the layout position for subsequent content. We already use a similar trick for ruby annotations, and some care is required so that we don't move back above the clearance offset when canceling ruby annotations overflow (which happens in some cases). There was no test coverage for this, so added three tests for that (they pass both and without this CL). Also added a multicol test that demonstrates what this fixes in column balancing. Bug: 829028 Change-Id: I84e58326e9ada78e3f29ad01d8043a173626aea3 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3168645 Commit-Queue: Morten Stenshorne <[email protected]> Reviewed-by: Ian Kilpatrick <[email protected]> Reviewed-by: Koji Ishii <[email protected]> Cr-Commit-Position: refs/heads/main@{#923502}
1 parent 2dc6651 commit faa9a85

File tree

4 files changed

+172
-0
lines changed

4 files changed

+172
-0
lines changed
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<!DOCTYPE html>
2+
<link rel="author" title="Morten Stenshorne" href="mailto:[email protected]">
3+
<link rel="help" href="https://www.w3.org/TR/css-multicol-1/#cf">
4+
<link rel="help" href="https://developer.mozilla.org/en-US/docs/Web/HTML/Element/br#deprecated_attributes">
5+
<p>Test passes if there is a filled green square below.</p>
6+
<div id="mc" style="columns:4; width:100px; background:green;">
7+
<div style="float:left; width:10px; height:400px;"></div>
8+
<br clear="all">
9+
</div>
10+
<script src="/resources/testharness.js"></script>
11+
<script src="/resources/testharnessreport.js"></script>
12+
<script>
13+
test(()=> {
14+
assert_equals(mc.offsetHeight, 100);
15+
});
16+
</script>

css/css-ruby/br-clear-all-000.html

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<!DOCTYPE html>
2+
<link rel="author" title="Morten Stenshorne" href="mailto:[email protected]">
3+
<link rel="help" href="https://www.w3.org/TR/css-ruby-1/#rubypos">
4+
<link rel="help" href="https://developer.mozilla.org/en-US/docs/Web/HTML/Element/br#deprecated_attributes">
5+
<style>
6+
#float {
7+
float: left;
8+
width: 100px;
9+
height: 100px;
10+
background: cyan;
11+
}
12+
#container {
13+
padding-bottom: 50px;
14+
line-height: 20px;
15+
background: yellow;
16+
}
17+
ruby {
18+
ruby-position: under;
19+
}
20+
ruby > div {
21+
display: inline-block;
22+
width: 20px;
23+
height: 20px;
24+
background: hotpink;
25+
}
26+
rt > div {
27+
display: inline-block;
28+
width: 50px;
29+
height: 50px;
30+
background: blue;
31+
}
32+
</style>
33+
<p>The yellow box should encompass its contents and also the cyan float, due to
34+
clearance. The yellow box has bottom padding, and the blue ruby annotation box
35+
is allowed to overflow into the padding area. In this case the blue box isn't
36+
tall enough to even get past the float, though.</p>
37+
<div id="float"></div>
38+
<div id="container" data-expected-height="150">
39+
<ruby>
40+
<div></div>
41+
<rt>
42+
<div></div>
43+
</rt>
44+
</ruby>
45+
<br clear="all">
46+
</div>
47+
<script src="/resources/testharness.js"></script>
48+
<script src="/resources/testharnessreport.js"></script>
49+
<script src="/resources/check-layout-th.js"></script>
50+
<script>
51+
checkLayout("#container");
52+
</script>

css/css-ruby/br-clear-all-001.html

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<!DOCTYPE html>
2+
<link rel="author" title="Morten Stenshorne" href="mailto:[email protected]">
3+
<link rel="help" href="https://www.w3.org/TR/css-ruby-1/#rubypos">
4+
<link rel="help" href="https://developer.mozilla.org/en-US/docs/Web/HTML/Element/br#deprecated_attributes">
5+
<style>
6+
#float {
7+
float: left;
8+
width: 100px;
9+
height: 100px;
10+
background: cyan;
11+
}
12+
#container {
13+
padding-bottom: 50px;
14+
line-height: 20px;
15+
background: yellow;
16+
}
17+
ruby {
18+
ruby-position: under;
19+
}
20+
ruby > div {
21+
display: inline-block;
22+
width: 20px;
23+
height: 20px;
24+
background: hotpink;
25+
}
26+
rt > div {
27+
display: inline-block;
28+
width: 50px;
29+
height: 100px;
30+
background: blue;
31+
}
32+
</style>
33+
<p>The yellow box should encompass its contents and also the cyan float, due to
34+
clearance. The yellow box has bottom padding, and the blue ruby annotation box
35+
is allowed to overflow into the padding area.</p>
36+
<div id="float"></div>
37+
<div id="container" data-expected-height="150">
38+
<ruby>
39+
<div></div>
40+
<rt>
41+
<div></div>
42+
</rt>
43+
</ruby>
44+
<br clear="all">
45+
</div>
46+
<script src="/resources/testharness.js"></script>
47+
<script src="/resources/testharnessreport.js"></script>
48+
<script src="/resources/check-layout-th.js"></script>
49+
<script>
50+
checkLayout("#container");
51+
</script>

css/css-ruby/br-clear-all-002.html

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
<!DOCTYPE html>
2+
<link rel="author" title="Morten Stenshorne" href="mailto:[email protected]">
3+
<link rel="help" href="https://www.w3.org/TR/css-ruby-1/#rubypos">
4+
<link rel="help" href="https://developer.mozilla.org/en-US/docs/Web/HTML/Element/br#deprecated_attributes">
5+
<style>
6+
#float {
7+
float: left;
8+
width: 100px;
9+
height: 100px;
10+
background: cyan;
11+
}
12+
#container {
13+
padding-bottom: 50px;
14+
line-height: 20px;
15+
background: yellow;
16+
}
17+
ruby {
18+
ruby-position: under;
19+
}
20+
ruby > div {
21+
display: inline-block;
22+
width: 20px;
23+
height: 20px;
24+
background: hotpink;
25+
}
26+
rt > div {
27+
display: inline-block;
28+
width: 50px;
29+
height: 150px;
30+
background: blue;
31+
}
32+
</style>
33+
<p>The yellow box should encompass its contents and also the cyan float, due to
34+
clearance. The yellow box has bottom padding, and the blue ruby annotation box
35+
is allowed to overflow into the padding area. In this case the blue box is so
36+
tall it will use the entire padding area, and also stretch the yellow box
37+
somewhat.</p>
38+
<div id="float"></div>
39+
<div id="container" data-expected-height="170">
40+
<ruby>
41+
<div></div>
42+
<rt>
43+
<div></div>
44+
</rt>
45+
</ruby>
46+
<br clear="all">
47+
</div>
48+
<script src="/resources/testharness.js"></script>
49+
<script src="/resources/testharnessreport.js"></script>
50+
<script src="/resources/check-layout-th.js"></script>
51+
<script>
52+
checkLayout("#container");
53+
</script>

0 commit comments

Comments
 (0)