Skip to content

Commit

Permalink
REGRESSION(r176978): Inline-blocks with overflowing contents have asc…
Browse files Browse the repository at this point in the history
…ents that are too large

https://bugs.webkit.org/show_bug.cgi?id=141783

Reviewed by David Hyatt.

Source/WebCore:

When we have an inline-block element, and we want to find its baseline (to lay out other
elements on the same line) we loop through the element's children and ask them what their
baselines are. The children use the location of the top of their last line to compute this
value. However, if the child has overflow-y, this might not be the correct calculation.

This behavior is in the spec: "The baseline of an 'inline-block' is the baseline of its last
line box in the normal flow, unless it has either no in-flow line boxes or if its 'overflow'
property has a computed value other than 'visible', in which case the baseline is the bottom
margin edge."
    -- http://www.w3.org/TR/CSS21/visudet.html#leading

However, we believe that a better policy is, when overflow is not "visible," to place the
baseline at the bottom of the block if the contents overflowed in the Y direction, and to place
it at the bottom of the last line if the contents did not overflow in the Y direction. This is
partially consistent with previous behavior, and isn't too far from the spec to cause too many
breakages.

Test: fast/css/inline-block-tricky-baselines.html
      fast/text/baseline-inline-block.html

* rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::inlineBlockBaseline):

LayoutTests:

Update expected results.

* css3/flexbox/child-overflow-expected.html:
* css3/flexbox/child-overflow.html:
* fast/css/inline-block-tricky-baselines-expected.html: Added.
* fast/css/inline-block-tricky-baselines.html: Added.
* fast/forms/textfield-overflow-by-value-update-expected.txt:
* fast/text/baseline-inline-block-expected.html: Added.
* fast/text/baseline-inline-block.html: Added.
* platform/mac/fast/forms/search-vertical-alignment-expected.txt:

Canonical link: https://commits.webkit.org/160545@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@181292 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
litherum committed Mar 9, 2015
1 parent 930c74a commit b181bc9
Show file tree
Hide file tree
Showing 12 changed files with 175 additions and 17 deletions.
18 changes: 18 additions & 0 deletions LayoutTests/ChangeLog
@@ -1,3 +1,21 @@
2015-03-09 Myles C. Maxfield <mmaxfield@apple.com>

REGRESSION(r176978): Inline-blocks with overflowing contents have ascents that are too large
https://bugs.webkit.org/show_bug.cgi?id=141783

Reviewed by David Hyatt.

Update expected results.

* css3/flexbox/child-overflow-expected.html:
* css3/flexbox/child-overflow.html:
* fast/css/inline-block-tricky-baselines-expected.html: Added.
* fast/css/inline-block-tricky-baselines.html: Added.
* fast/forms/textfield-overflow-by-value-update-expected.txt:
* fast/text/baseline-inline-block-expected.html: Added.
* fast/text/baseline-inline-block.html: Added.
* platform/mac/fast/forms/search-vertical-alignment-expected.txt:

2015-03-09 Andy Estes <aestes@apple.com>

[Content Filtering] Add tests
Expand Down
1 change: 1 addition & 0 deletions LayoutTests/css3/flexbox/child-overflow-expected.html
Expand Up @@ -4,6 +4,7 @@
<style>
.container {
display: inline-block;
vertical-align: top;
margin-right: 30px;
width: 100px;
height: 100px;
Expand Down
1 change: 1 addition & 0 deletions LayoutTests/css3/flexbox/child-overflow.html
Expand Up @@ -5,6 +5,7 @@
<style>
.container {
display: inline-block;
vertical-align: top;
margin-right: 30px;
}

Expand Down
24 changes: 24 additions & 0 deletions LayoutTests/fast/css/inline-block-tricky-baselines-expected.html
@@ -0,0 +1,24 @@
<!DOCTYPE html>
<style>

.inline-block {
display: inline-block;
}

.inner {
display: block;
width: 22px;
height: 13px;
overflow: hidden;
}
</style>
<div class="pause inline-block">
<div class="inner">
Single
</div>
</div>
<div class="next inline-block">
<div class="inner">
Multiple
</div>
</div>
24 changes: 24 additions & 0 deletions LayoutTests/fast/css/inline-block-tricky-baselines.html
@@ -0,0 +1,24 @@
<!DOCTYPE html>
<style>

.inline-block {
display: inline-block;
}

.inner {
display: block;
width: 22px;
height: 13px;
overflow: hidden;
}
</style>
<div class="pause inline-block">
<div class="inner">
Single-line
</div>
</div>
<div class="next inline-block">
<div class="inner">
Multiple words that get wrapped
</div>
</div>
@@ -1,8 +1,8 @@
layer at (0,0) size 800x600
RenderView at (0,0) size 800x600
layer at (0,0) size 800x52
RenderBlock {HTML} at (0,0) size 800x52
RenderBody {BODY} at (8,8) size 784x36
RenderTextControl {INPUT} at (0,0) size 102x2 [bgcolor=#FFFFFF]
layer at (0,0) size 800x34
RenderBlock {HTML} at (0,0) size 800x34
RenderBody {BODY} at (8,8) size 784x18
RenderTextControl {INPUT} at (0,13) size 102x2 [bgcolor=#FFFFFF]
RenderText {#text} at (0,0) size 0x0
RenderText {#text} at (0,0) size 0x0
15 changes: 15 additions & 0 deletions LayoutTests/fast/text/baseline-inline-block-expected.html
@@ -0,0 +1,15 @@
<!DOCTYPE html>
<html>
<body>
<div style="background: cyan; width: 100%; height: 352px;"></div>
<div style="background: green; position: absolute; left: 13px; top: 95px; width: 50px; height: 150px; overflow: scroll;"><div style="width: 16px; height: 240px; background: black;"></div></div>
<div style="background: green; position: absolute; left: 84px; top: 90px; width: 60px; height: 160px;"><div style="background: green; position: absolute; left: 5px; top: 5px; width: 50px; height: 150px; overflow: scroll;"><div style="width: 16px; height: 240px; background: black;"></div></div></div>
<div style="background: green; position: absolute; left: 165px; top: 13px; width: 50px; height: 150px;"><div style="width: 16px; height: 240px; background: black;"></div></div>
<div style="background: green; position: absolute; left: 236px; top: 8px; width: 60px; height: 160px;"><div style="background: green; position: absolute; left: 5px; top: 5px; width: 50px; height: 150px;"><div style="width: 16px; height: 240px; background: black;"></div></div></div>
<div style="background: green; position: absolute; left: 312px; top: 184px; width: 60px; height: 160px;"><div style="background: green; position: absolute; left: 5px; top: 5px; width: 50px; height: 150px; overflow: scroll;"><div style="width: 16px; height: 64px; background: black;"></div></div></div>
<div style="background: green; position: absolute; left: 388px; top: 184px; width: 60px; height: 160px;"><div style="background: green; position: absolute; left: 5px; top: 5px; width: 50px; height: 150px;"><div style="width: 16px; height: 64px; background: black;"></div></div></div>
<div style="background: green; position: absolute; left: 469px; top: 189px; width: 50px; height: 150px;"><div style="width: 16px; height: 64px; background: black;"></div></div>
<div style="background: black; position: absolute; left: 540px; top: 237px; width: 16px; height: 16px;"></div>
<div style="background: black; position: absolute; left: 8px; top: 344px; width: 16px; height: 16px;"></div>
</body>
</html>
37 changes: 37 additions & 0 deletions LayoutTests/fast/text/baseline-inline-block.html
@@ -0,0 +1,37 @@
<!DOCTYPE html>
<html>
<body>
<div style="background: cyan; font-family: Ahem; -webkit-font-smoothing: none;">
<div style="display: inline-block; background: green; height: 150px; width: 50px; overflow: scroll; margin: 5px;">
L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>
</div>
<div style="display: inline-block; background: green;">
<div style="height: 150px; width: 50px; overflow: scroll; margin: 5px;">
L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>
</div>
</div>
<div style="display: inline-block; background: green; height: 150px; width: 50px; margin: 5px;">
L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>
</div>
<div style="display: inline-block; background: green;">
<div style="height: 150px; width: 50px; margin: 5px;">
L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>L<br>
</div>
</div>
<div style="display: inline-block; background: green;">
<div style="height: 150px; width: 50px; overflow: scroll; margin: 5px;">
L<br>L<br>L<br>L<br>
</div>
</div>
<div style="display: inline-block; background: green;">
<div style="height: 150px; width: 50px; margin: 5px;">
L<br>L<br>L<br>L<br>
</div>
</div>
<div style="display: inline-block; background: green; height: 150px; width: 50px; margin: 5px;">
L<br>L<br>L<br>L<br>
</div>
L<br>L
</div>
</body>
</html>
Expand Up @@ -31,14 +31,14 @@ layer at (0,0) size 800x600
RenderTextControl {INPUT} at (185,2) size 146x16 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
RenderText {#text} at (0,0) size 0x0
RenderBlock {P} at (0,147) size 784x18
RenderTextControl {INPUT} at (2,0) size 176x12 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
RenderTextControl {INPUT} at (2,5) size 176x12 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
RenderFlexibleBox {DIV} at (3,0) size 170x12
RenderBlock {DIV} at (0,0) size 17x12
RenderBlock {DIV} at (17,3) size 140x6
RenderBlock {DIV} at (156,0) size 14x12
RenderText {#text} at (179,0) size 5x18
text run at (179,0) width 5: " "
RenderTextControl {INPUT} at (185,0) size 146x12 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
RenderTextControl {INPUT} at (185,5) size 146x12 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
RenderText {#text} at (0,0) size 0x0
layer at (30,76) size 139x13
RenderBlock {DIV} at (0,0) size 140x13
Expand All @@ -56,11 +56,11 @@ layer at (196,124) size 139x13
RenderBlock {DIV} at (3,1) size 140x13
RenderText {#text} at (0,0) size 23x13
text run at (0,0) width 23: "Text"
layer at (30,158) size 139x6 scrollHeight 13
layer at (30,163) size 139x6 scrollHeight 13
RenderBlock {DIV} at (0,0) size 140x6
RenderText {#text} at (0,0) size 23x13
text run at (0,0) width 23: "Text"
layer at (196,158) size 139x6 scrollHeight 13
layer at (196,163) size 139x6 scrollHeight 13
RenderBlock {DIV} at (3,3) size 140x6
RenderText {#text} at (0,0) size 23x13
text run at (0,0) width 23: "Text"
Expand Up @@ -31,14 +31,14 @@ layer at (0,0) size 800x600
RenderTextControl {INPUT} at (176,2) size 137x16 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
RenderText {#text} at (0,0) size 0x0
RenderBlock {P} at (0,147) size 784x18
RenderTextControl {INPUT} at (2,0) size 167x12 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
RenderTextControl {INPUT} at (2,5) size 167x12 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
RenderFlexibleBox {DIV} at (3,0) size 161x12
RenderBlock {DIV} at (0,0) size 17x12
RenderBlock {DIV} at (17,3) size 131x6
RenderBlock {DIV} at (147,0) size 14x12
RenderText {#text} at (170,0) size 5x18
text run at (170,0) width 5: " "
RenderTextControl {INPUT} at (176,0) size 137x12 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
RenderTextControl {INPUT} at (176,5) size 137x12 [bgcolor=#FFFFFF] [border: (2px inset #000000)]
RenderText {#text} at (0,0) size 0x0
layer at (30,76) size 130x13
RenderBlock {DIV} at (0,0) size 131x13
Expand All @@ -56,11 +56,11 @@ layer at (187,124) size 130x13
RenderBlock {DIV} at (3,1) size 131x13
RenderText {#text} at (0,0) size 22x13
text run at (0,0) width 22: "Text"
layer at (30,158) size 130x6 scrollHeight 13
layer at (30,163) size 130x6 scrollHeight 13
RenderBlock {DIV} at (0,0) size 131x6
RenderText {#text} at (0,0) size 22x13
text run at (0,0) width 22: "Text"
layer at (187,158) size 130x6 scrollHeight 13
layer at (187,163) size 130x6 scrollHeight 13
RenderBlock {DIV} at (3,3) size 131x6
RenderText {#text} at (0,0) size 22x13
text run at (0,0) width 22: "Text"
30 changes: 30 additions & 0 deletions Source/WebCore/ChangeLog
@@ -1,3 +1,33 @@
2015-03-09 Myles C. Maxfield <mmaxfield@apple.com>

REGRESSION(r176978): Inline-blocks with overflowing contents have ascents that are too large
https://bugs.webkit.org/show_bug.cgi?id=141783

Reviewed by David Hyatt.

When we have an inline-block element, and we want to find its baseline (to lay out other
elements on the same line) we loop through the element's children and ask them what their
baselines are. The children use the location of the top of their last line to compute this
value. However, if the child has overflow-y, this might not be the correct calculation.

This behavior is in the spec: "The baseline of an 'inline-block' is the baseline of its last
line box in the normal flow, unless it has either no in-flow line boxes or if its 'overflow'
property has a computed value other than 'visible', in which case the baseline is the bottom
margin edge."
-- http://www.w3.org/TR/CSS21/visudet.html#leading

However, we believe that a better policy is, when overflow is not "visible," to place the
baseline at the bottom of the block if the contents overflowed in the Y direction, and to place
it at the bottom of the last line if the contents did not overflow in the Y direction. This is
partially consistent with previous behavior, and isn't too far from the spec to cause too many
breakages.

Test: fast/css/inline-block-tricky-baselines.html
fast/text/baseline-inline-block.html

* rendering/RenderBlockFlow.cpp:
(WebCore::RenderBlockFlow::inlineBlockBaseline):

2015-03-09 Andy Estes <aestes@apple.com>

[Content Filtering] Add tests
Expand Down
18 changes: 13 additions & 5 deletions Source/WebCore/rendering/RenderBlockFlow.cpp
Expand Up @@ -3023,12 +3023,20 @@ int RenderBlockFlow::inlineBlockBaseline(LineDirectionMode lineDirection) const
+ (lineDirection == HorizontalLine ? borderTop() + paddingTop() : borderRight() + paddingRight());
}

// Note that here we only take the left and bottom into consideration. Our caller takes the right and top into consideration.
float boxHeight = lineDirection == HorizontalLine ? height() + m_marginBox.bottom() : width() + m_marginBox.left();
float lastBaseline;
if (auto simpleLineLayout = this->simpleLineLayout())
return SimpleLineLayout::computeFlowLastLineBaseline(*this, *simpleLineLayout);

bool isFirstLine = lastRootBox() == firstRootBox();
const RenderStyle& style = isFirstLine ? firstLineStyle() : this->style();
return lastRootBox()->logicalTop() + style.fontMetrics().ascent(lastRootBox()->baselineType());
lastBaseline = SimpleLineLayout::computeFlowLastLineBaseline(*this, *simpleLineLayout);
else {
bool isFirstLine = lastRootBox() == firstRootBox();
const RenderStyle& style = isFirstLine ? firstLineStyle() : this->style();
lastBaseline = lastRootBox()->logicalTop() + style.fontMetrics().ascent(lastRootBox()->baselineType());
}
// According to the CSS spec http://www.w3.org/TR/CSS21/visudet.html, we shouldn't be performing this min, but should
// instead be returning boxHeight directly. However, we feel that a min here is better behavior (and is consistent
// enough with the spec to not cause tons of breakages).
return style().overflowY() == OVISIBLE ? lastBaseline : std::min(boxHeight, lastBaseline);
}

GapRects RenderBlockFlow::inlineSelectionGaps(RenderBlock& rootBlock, const LayoutPoint& rootBlockPhysicalPosition, const LayoutSize& offsetFromRootBlock,
Expand Down

0 comments on commit b181bc9

Please sign in to comment.