- Revision
- 290105
- Author
- [email protected]
- Date
- 2022-02-17 18:51:32 -0800 (Thu, 17 Feb 2022)
Log Message
[LFC][IFC] 'vertical-align: bottom' makes inline level boxes stick out of their parent block box
https://bugs.webkit.org/show_bug.cgi?id=236677
Reviewed by Antti Koivisto.
Source/WebCore:
'vertical-align: bottom' pushes the root inline box's baseline down as it stretches the line box downwards.
In most cases this makes the root inline box sit at the bottom of the line box e.g.
<div>
some text
<img style="vertical-align: bottom; height: 100px; width: 10px;">
</div>
forms a 100px tall block box where the "some text" sits at the bottom of the line and the tall image fills the rest of the line box all the way to the top.
Now if we add another inline level box with e.g. middle alignment.
<div>
some text
<img style="vertical-align: middle; height: 80px; width: 10px;">
<img style="vertical-align: bottom; height: 100px; width: 10px;">
</div>
We still form a 100px tall block box where the "some text" content sits at the bottom and the middle aligned box stick out of the block box by about 40px (middle alignment -> 80px/2) as it is aligned based
on the root inline box's position (which is at the very bottom of the block container).
Instead the root inline box's baseline should be pull up to make room for the middle aligned image box.
This patch fixes this incorrect behavior by removing the line box relative alignment types from the regular alignment logic and offsetting the root inline box's baseline position at a later step.
What we do here is we simply compute the root inline box's baseline position as if there were no line box relative aligned boxes present and offset the final result if such box happens to stretch the line box.
(going back to the example: without the tall, bottom aligned image box, we compute the root inline box's baseline position to be around 40px to
ensure that the 80px image box fits and later we push it down to 60px (as the 100px image box stretches the line box by 20px).)
Test: fast/inline/vertical-align-bottom.html
* layout/formattingContexts/inline/InlineLineBoxVerticalAligner.cpp:
(WebCore::Layout::LineBoxVerticalAligner::computeLogicalHeightAndAlign const):
(WebCore::Layout::LineBoxVerticalAligner::computeLineBoxLogicalHeight const):
(WebCore::Layout::LineBoxVerticalAligner::computeRootInlineBoxVerticalPosition const):
* layout/formattingContexts/inline/InlineLineBoxVerticalAligner.h:
(WebCore::Layout::LineBoxVerticalAligner::LineBoxHeight::value const):
LayoutTests:
* fast/inline/vertical-align-bottom-expected.html: Added.
* fast/inline/vertical-align-bottom.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (290104 => 290105)
--- trunk/LayoutTests/ChangeLog 2022-02-18 02:46:22 UTC (rev 290104)
+++ trunk/LayoutTests/ChangeLog 2022-02-18 02:51:32 UTC (rev 290105)
@@ -1,3 +1,13 @@
+2022-02-17 Alan Bujtas <[email protected]>
+
+ [LFC][IFC] 'vertical-align: bottom' makes inline level boxes stick out of their parent block box
+ https://bugs.webkit.org/show_bug.cgi?id=236677
+
+ Reviewed by Antti Koivisto.
+
+ * fast/inline/vertical-align-bottom-expected.html: Added.
+ * fast/inline/vertical-align-bottom.html: Added.
+
2022-02-17 Jon Lee <[email protected]>
Unreviewed gardening for iOS GPU Process bots.
Added: trunk/LayoutTests/fast/inline/vertical-align-bottom-expected.html (0 => 290105)
--- trunk/LayoutTests/fast/inline/vertical-align-bottom-expected.html (rev 0)
+++ trunk/LayoutTests/fast/inline/vertical-align-bottom-expected.html 2022-02-18 02:51:32 UTC (rev 290105)
@@ -0,0 +1,15 @@
+<!DOCTYPE html>
+<style>
+div {
+ background-color: green;
+ width: 120px;
+ margin-bottom: 20px;
+}
+
+</style>
+<div style="height: 50px;"></div>
+<div style="height: 100px;"></div>
+<div style="height: 200px;"></div>
+<div style="height: 50px;"></div>
+<div style="height: 50px;"></div>
+<div style="height: 50px;"></div>
\ No newline at end of file
Added: trunk/LayoutTests/fast/inline/vertical-align-bottom.html (0 => 290105)
--- trunk/LayoutTests/fast/inline/vertical-align-bottom.html (rev 0)
+++ trunk/LayoutTests/fast/inline/vertical-align-bottom.html 2022-02-18 02:51:32 UTC (rev 290105)
@@ -0,0 +1,42 @@
+<!DOCTYPE html>
+<style>
+.block {
+ background-color: green;
+ width: 120px;
+ margin-bottom: 20px;
+ color: transparent;
+ font-family: Ahem;
+ font-size: 20px;
+}
+
+.aligned-box {
+ display: inline-block;
+ background-color: green;
+ width: 50px;
+ height: 50px;
+}
+
+.non-lineBox-relative {
+ vertical-align: baseline;
+ width: 50px;
+ height: 20px;
+}
+
+.bottom {
+ vertical-align: bottom;
+
+ width: 50px;
+ height: 50px;
+}
+
+.top {
+ vertical-align: top;
+}
+</style>
+<!-- The non-line-box relative inline level boxes should not stick out of their parent block box -->
+<div class=block>X<div class="aligned-box non-lineBox-relative">X</div><div class="aligned-box bottom"></div></div>
+<div class=block>X<div class="aligned-box non-lineBox-relative" style="height: 100px;">X</div><div class="aligned-box bottom"></div></div>
+<div class=block>X<div class="aligned-box non-lineBox-relative" style="padding-bottom: 100px; height: 100px;">X</div><div class="aligned-box bottom"></div></div>
+<div class=block>X<div class="aligned-box non-lineBox-relative">X</div><div class="aligned-box bottom"></div></div>
+<div class=block>X<div class="aligned-box non-lineBox-relative" style="height: 30px; vertical-align: middle">X</div><div class="aligned-box bottom"></div></div>
+<div class=block>X<div class="aligned-box non-lineBox-relative">X</div><div class="aligned-box top"></div></div>
Modified: trunk/Source/WebCore/ChangeLog (290104 => 290105)
--- trunk/Source/WebCore/ChangeLog 2022-02-18 02:46:22 UTC (rev 290104)
+++ trunk/Source/WebCore/ChangeLog 2022-02-18 02:51:32 UTC (rev 290105)
@@ -1,3 +1,42 @@
+2022-02-17 Alan Bujtas <[email protected]>
+
+ [LFC][IFC] 'vertical-align: bottom' makes inline level boxes stick out of their parent block box
+ https://bugs.webkit.org/show_bug.cgi?id=236677
+
+ Reviewed by Antti Koivisto.
+
+ 'vertical-align: bottom' pushes the root inline box's baseline down as it stretches the line box downwards.
+ In most cases this makes the root inline box sit at the bottom of the line box e.g.
+ <div>
+ some text
+ <img style="vertical-align: bottom; height: 100px; width: 10px;">
+ </div>
+ forms a 100px tall block box where the "some text" sits at the bottom of the line and the tall image fills the rest of the line box all the way to the top.
+
+ Now if we add another inline level box with e.g. middle alignment.
+ <div>
+ some text
+ <img style="vertical-align: middle; height: 80px; width: 10px;">
+ <img style="vertical-align: bottom; height: 100px; width: 10px;">
+ </div>
+ We still form a 100px tall block box where the "some text" content sits at the bottom and the middle aligned box stick out of the block box by about 40px (middle alignment -> 80px/2) as it is aligned based
+ on the root inline box's position (which is at the very bottom of the block container).
+ Instead the root inline box's baseline should be pull up to make room for the middle aligned image box.
+
+ This patch fixes this incorrect behavior by removing the line box relative alignment types from the regular alignment logic and offsetting the root inline box's baseline position at a later step.
+ What we do here is we simply compute the root inline box's baseline position as if there were no line box relative aligned boxes present and offset the final result if such box happens to stretch the line box.
+ (going back to the example: without the tall, bottom aligned image box, we compute the root inline box's baseline position to be around 40px to
+ ensure that the 80px image box fits and later we push it down to 60px (as the 100px image box stretches the line box by 20px).)
+
+ Test: fast/inline/vertical-align-bottom.html
+
+ * layout/formattingContexts/inline/InlineLineBoxVerticalAligner.cpp:
+ (WebCore::Layout::LineBoxVerticalAligner::computeLogicalHeightAndAlign const):
+ (WebCore::Layout::LineBoxVerticalAligner::computeLineBoxLogicalHeight const):
+ (WebCore::Layout::LineBoxVerticalAligner::computeRootInlineBoxVerticalPosition const):
+ * layout/formattingContexts/inline/InlineLineBoxVerticalAligner.h:
+ (WebCore::Layout::LineBoxVerticalAligner::LineBoxHeight::value const):
+
2022-02-17 Kate Cheney <[email protected]>
Refactor share menu item presentation
Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxVerticalAligner.cpp (290104 => 290105)
--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxVerticalAligner.cpp 2022-02-18 02:46:22 UTC (rev 290104)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxVerticalAligner.cpp 2022-02-18 02:51:32 UTC (rev 290105)
@@ -83,9 +83,9 @@
// 3. Finally align the inline level boxes using (mostly) normal inline level box geometries.
ASSERT(lineBox.hasContent());
auto lineBoxLogicalHeight = computeLineBoxLogicalHeight(lineBox);
- computeRootInlineBoxVerticalPosition(lineBox);
- alignInlineLevelBoxes(lineBox, lineBoxLogicalHeight);
- return lineBoxLogicalHeight;
+ computeRootInlineBoxVerticalPosition(lineBox, lineBoxLogicalHeight);
+ alignInlineLevelBoxes(lineBox, lineBoxLogicalHeight.value());
+ return lineBoxLogicalHeight.value();
}
InlineLayoutUnit LineBoxVerticalAligner::simplifiedVerticalAlignment(LineBox& lineBox) const
@@ -117,7 +117,7 @@
return lineBoxLogicalBottom - lineBoxLogicalTop;
}
-InlineLayoutUnit LineBoxVerticalAligner::computeLineBoxLogicalHeight(LineBox& lineBox) const
+LineBoxVerticalAligner::LineBoxHeight LineBoxVerticalAligner::computeLineBoxLogicalHeight(LineBox& lineBox) const
{
// This function (partially) implements:
// 2.2. Layout Within Line Boxes
@@ -217,20 +217,31 @@
// The line box height computation is as follows:
// 1. Stretch the line box with the non-line-box relative aligned inline box absolute top and bottom values.
// 2. Check if the line box relative aligned inline boxes (top, bottom etc) have enough room and stretch the line box further if needed.
- auto lineBoxLogicalHeight = valueOrDefault(maximumLogicalBottom) - valueOrDefault(minimumLogicalTop);
+ auto nonBottomAlignedBoxesMaximumHeight = valueOrDefault(maximumLogicalBottom) - valueOrDefault(minimumLogicalTop);
+ auto bottomAlignedBoxesMaximumHeight = std::optional<InlineLayoutUnit> { };
for (auto* lineBoxRelativeInlineLevelBox : lineBoxRelativeInlineLevelBoxes) {
if (!formattingGeometry.inlineLevelBoxAffectsLineBox(*lineBoxRelativeInlineLevelBox, lineBox))
continue;
- lineBoxLogicalHeight = std::max(lineBoxLogicalHeight, lineBoxRelativeInlineLevelBox->layoutBounds().height());
+ // This line box relative aligned inline level box stretches the line box.
+ auto inlineLevelBoxHeight = lineBoxRelativeInlineLevelBox->layoutBounds().height();
+ if (lineBoxRelativeInlineLevelBox->verticalAlign().type == VerticalAlign::Top) {
+ nonBottomAlignedBoxesMaximumHeight = std::max(inlineLevelBoxHeight, nonBottomAlignedBoxesMaximumHeight);
+ continue;
+ }
+ if (lineBoxRelativeInlineLevelBox->verticalAlign().type == VerticalAlign::Bottom) {
+ bottomAlignedBoxesMaximumHeight = std::max(inlineLevelBoxHeight, bottomAlignedBoxesMaximumHeight.value_or(0.f));
+ continue;
+ }
+ ASSERT_NOT_REACHED();
}
- return lineBoxLogicalHeight;
+ return { nonBottomAlignedBoxesMaximumHeight, bottomAlignedBoxesMaximumHeight };
}
-void LineBoxVerticalAligner::computeRootInlineBoxVerticalPosition(LineBox& lineBox) const
+void LineBoxVerticalAligner::computeRootInlineBoxVerticalPosition(LineBox& lineBox, const LineBoxHeight& lineBoxHeight) const
{
auto& rootInlineBox = lineBox.rootInlineBox();
auto& formattingGeometry = this->formattingGeometry();
- Vector<unsigned> inlineLevelBoxIndexesWithLineBoxRelativeAlignment;
+ auto hasTopAlignedInlineLevelBox = false;
HashMap<const InlineLevelBox*, InlineLayoutUnit> inlineLevelBoxAbsoluteBaselineOffsetMap;
inlineLevelBoxAbsoluteBaselineOffsetMap.add(&rootInlineBox, InlineLayoutUnit { });
@@ -250,13 +261,13 @@
auto verticalAlign = inlineLevelBox.verticalAlign();
if (inlineLevelBox.hasLineBoxRelativeAlignment()) {
- if (verticalAlign.type == VerticalAlign::Top)
+ if (verticalAlign.type == VerticalAlign::Top) {
+ hasTopAlignedInlineLevelBox = hasTopAlignedInlineLevelBox || affectsRootInlineBoxVerticalPosition(inlineLevelBox);
inlineLevelBoxAbsoluteBaselineOffsetMap.add(&inlineLevelBox, rootInlineBox.layoutBounds().ascent - inlineLevelBox.layoutBounds().ascent);
- else if (verticalAlign.type == VerticalAlign::Bottom)
+ } else if (verticalAlign.type == VerticalAlign::Bottom)
inlineLevelBoxAbsoluteBaselineOffsetMap.add(&inlineLevelBox, inlineLevelBox.layoutBounds().descent - rootInlineBox.layoutBounds().descent);
else
ASSERT_NOT_REACHED();
- inlineLevelBoxIndexesWithLineBoxRelativeAlignment.append(index);
continue;
}
auto& layoutBox = inlineLevelBox.layoutBox();
@@ -315,24 +326,20 @@
}
}
- for (auto index : inlineLevelBoxIndexesWithLineBoxRelativeAlignment) {
- auto& inlineLevelBox = nonRootInlineLevelBoxes[index];
- if (!affectsRootInlineBoxVerticalPosition(inlineLevelBox))
- continue;
-
- if (inlineLevelBox.verticalAlign().type == VerticalAlign::Bottom) {
- auto baselineOffset = inlineLevelBox.layoutBounds().descent - rootInlineBox.layoutBounds().descent;
- auto topOffsetFromRootInlineBoxBaseline = baselineOffset + inlineLevelBox.layoutBounds().ascent;
- maximumTopOffsetFromRootInlineBoxBaseline = std::max(*maximumTopOffsetFromRootInlineBoxBaseline, topOffsetFromRootInlineBoxBaseline);
- } else if (inlineLevelBox.verticalAlign().type == VerticalAlign::Top) {
- // vertical-align: top is a line box relative alignment. It stretches the line box downwards meaning that it does not affect
- // the root inline box's baseline position, but in quirks mode we have to ensure that the root inline box does not end up at 0px.
- if (!maximumTopOffsetFromRootInlineBoxBaseline)
- maximumTopOffsetFromRootInlineBoxBaseline = rootInlineBox.ascent();
- } else
- ASSERT_NOT_REACHED();
+ if (!maximumTopOffsetFromRootInlineBoxBaseline && hasTopAlignedInlineLevelBox) {
+ // vertical-align: top is a line box relative alignment. It stretches the line box downwards meaning that it does not affect
+ // the root inline box's baseline position, but in quirks mode we have to ensure that the root inline box does not end up at 0px.
+ maximumTopOffsetFromRootInlineBoxBaseline = rootInlineBox.ascent();
}
- auto rootInlineBoxLogicalTop = maximumTopOffsetFromRootInlineBoxBaseline.value_or(0.f) - rootInlineBox.ascent();
+ // vertical-align: bottom stretches the top of the line box pushing the root inline box downwards.
+ auto bottomAlignedBoxStretch = InlineLayoutUnit { };
+ if (lineBoxHeight.bottomAlignedBoxesMaximumHeight) {
+ // Negative vertical margin can make aligned boxes have negative height.
+ bottomAlignedBoxStretch = std::max(0.f, lineBoxHeight.nonBottomAlignedBoxesMaximumHeight < 0
+ ? *lineBoxHeight.bottomAlignedBoxesMaximumHeight
+ : std::max(0.f, *lineBoxHeight.bottomAlignedBoxesMaximumHeight - lineBoxHeight.nonBottomAlignedBoxesMaximumHeight));
+ }
+ auto rootInlineBoxLogicalTop = bottomAlignedBoxStretch + maximumTopOffsetFromRootInlineBoxBaseline.value_or(0.f) - rootInlineBox.ascent();
rootInlineBox.setLogicalTop(rootInlineBoxLogicalTop);
}
Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxVerticalAligner.h (290104 => 290105)
--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxVerticalAligner.h 2022-02-18 02:46:22 UTC (rev 290104)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxVerticalAligner.h 2022-02-18 02:51:32 UTC (rev 290105)
@@ -41,8 +41,14 @@
private:
InlineLayoutUnit simplifiedVerticalAlignment(LineBox&) const;
- InlineLayoutUnit computeLineBoxLogicalHeight(LineBox&) const;
- void computeRootInlineBoxVerticalPosition(LineBox&) const;
+ struct LineBoxHeight {
+ InlineLayoutUnit value() const { return std::max(nonBottomAlignedBoxesMaximumHeight, bottomAlignedBoxesMaximumHeight.value_or(0.f)); }
+
+ InlineLayoutUnit nonBottomAlignedBoxesMaximumHeight { 0 };
+ std::optional<InlineLayoutUnit> bottomAlignedBoxesMaximumHeight { };
+ };
+ LineBoxHeight computeLineBoxLogicalHeight(LineBox&) const;
+ void computeRootInlineBoxVerticalPosition(LineBox&, const LineBoxHeight&) const;
void alignInlineLevelBoxes(LineBox&, InlineLayoutUnit lineBoxLogicalHeight) const;
const InlineFormattingGeometry& formattingGeometry() const { return m_inlineFormattingGeometry; }
