- Revision
- 238735
- Author
- [email protected]
- Date
- 2018-11-30 07:22:39 -0800 (Fri, 30 Nov 2018)
Log Message
[LFC][BFC][MarginCollapsing] Do not use computed display box values for border and padding
https://bugs.webkit.org/show_bug.cgi?id=192214
Reviewed by Antti Koivisto.
Source/WebCore:
Border and padding values are not necessarily computed yet when we try to estimate the margin top value. Estimating margin top is required
to be able to place floats (vertically) sooner than we would compute the final vertical position for a regular block box.
<body><div style="float: left"></div><div><div style="margin: 10px;"></div></div>
In the above example, to estimate a final vertical position of the floating box, we need to know whether the nested div's margin is collapsed
all the way up to the body. However in order to find it out we need to check for borders and paddings (they stop margin collapsing).
At the time when the floating box is being laied out, those <div> block boxes are still far down in the layout queue.
* layout/blockformatting/BlockFormattingContext.h:
* layout/blockformatting/BlockFormattingContextGeometry.cpp:
(WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMargin):
* layout/blockformatting/BlockMarginCollapse.cpp:
(WebCore::Layout::hasBorder):
(WebCore::Layout::hasPadding):
(WebCore::Layout::hasBorderBefore):
(WebCore::Layout::hasBorderAfter):
(WebCore::Layout::hasPaddingBefore):
(WebCore::Layout::hasPaddingAfter):
(WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::isMarginTopCollapsedWithParent):
(WebCore::Layout::isMarginBottomCollapsedThrough):
(WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::marginTop):
(WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::marginBottom):
(WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::isMarginBottomCollapsedWithParent):
(WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::collapsedMarginBottomFromLastChild):
Tools:
* LayoutReloaded/misc/LFC-passing-tests.txt:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (238734 => 238735)
--- trunk/Source/WebCore/ChangeLog 2018-11-30 14:52:43 UTC (rev 238734)
+++ trunk/Source/WebCore/ChangeLog 2018-11-30 15:22:39 UTC (rev 238735)
@@ -1,3 +1,36 @@
+2018-11-30 Zalan Bujtas <[email protected]>
+
+ [LFC][BFC][MarginCollapsing] Do not use computed display box values for border and padding
+ https://bugs.webkit.org/show_bug.cgi?id=192214
+
+ Reviewed by Antti Koivisto.
+
+ Border and padding values are not necessarily computed yet when we try to estimate the margin top value. Estimating margin top is required
+ to be able to place floats (vertically) sooner than we would compute the final vertical position for a regular block box.
+
+ <body><div style="float: left"></div><div><div style="margin: 10px;"></div></div>
+
+ In the above example, to estimate a final vertical position of the floating box, we need to know whether the nested div's margin is collapsed
+ all the way up to the body. However in order to find it out we need to check for borders and paddings (they stop margin collapsing).
+ At the time when the floating box is being laied out, those <div> block boxes are still far down in the layout queue.
+
+ * layout/blockformatting/BlockFormattingContext.h:
+ * layout/blockformatting/BlockFormattingContextGeometry.cpp:
+ (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMargin):
+ * layout/blockformatting/BlockMarginCollapse.cpp:
+ (WebCore::Layout::hasBorder):
+ (WebCore::Layout::hasPadding):
+ (WebCore::Layout::hasBorderBefore):
+ (WebCore::Layout::hasBorderAfter):
+ (WebCore::Layout::hasPaddingBefore):
+ (WebCore::Layout::hasPaddingAfter):
+ (WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::isMarginTopCollapsedWithParent):
+ (WebCore::Layout::isMarginBottomCollapsedThrough):
+ (WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::marginTop):
+ (WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::marginBottom):
+ (WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::isMarginBottomCollapsedWithParent):
+ (WebCore::Layout::BlockFormattingContext::Geometry::MarginCollapse::collapsedMarginBottomFromLastChild):
+
2018-11-29 Frederic Wang <[email protected]>
Separate paint and scroll offsets for RenderLayerBacking::m_scrollingContentsLayer
Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h (238734 => 238735)
--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h 2018-11-30 14:52:43 UTC (rev 238734)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h 2018-11-30 15:22:39 UTC (rev 238735)
@@ -89,7 +89,7 @@
static LayoutUnit marginTop(const LayoutState&, const Box&);
static LayoutUnit marginBottom(const LayoutState&, const Box&);
- static bool isMarginBottomCollapsedWithParent(const LayoutState&, const Box&);
+ static bool isMarginBottomCollapsedWithParent(const Box&);
static bool isMarginTopCollapsedWithParentMarginBottom(const Box&);
private:
Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp (238734 => 238735)
--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp 2018-11-30 14:52:43 UTC (rev 238734)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp 2018-11-30 15:22:39 UTC (rev 238735)
@@ -83,7 +83,7 @@
// 2. the bottom edge of the bottom (possibly collapsed) margin of its last in-flow child, if the child's bottom margin...
auto* lastInFlowChild = downcast<Container>(layoutBox).lastInFlowChild();
ASSERT(lastInFlowChild);
- if (!MarginCollapse::isMarginBottomCollapsedWithParent(layoutState, *lastInFlowChild)) {
+ if (!MarginCollapse::isMarginBottomCollapsedWithParent(*lastInFlowChild)) {
auto& lastInFlowDisplayBox = layoutState.displayBoxForLayoutBox(*lastInFlowChild);
return { lastInFlowDisplayBox.bottom() + lastInFlowDisplayBox.marginBottom() - borderAndPaddingTop, nonCollapsedMargin, collapsedMargin };
}
Modified: trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp (238734 => 238735)
--- trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp 2018-11-30 14:52:43 UTC (rev 238734)
+++ trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp 2018-11-30 15:22:39 UTC (rev 238735)
@@ -36,6 +36,39 @@
namespace WebCore {
namespace Layout {
+static bool hasBorder(const BorderValue& borderValue)
+{
+ if (borderValue.style() == BorderStyle::None || borderValue.style() == BorderStyle::Hidden)
+ return false;
+ return !!borderValue.width();
+}
+
+static bool hasPadding(const Length& paddingValue)
+{
+ // FIXME: Check if percent value needs to be resolved.
+ return !paddingValue.isZero();
+}
+
+static bool hasBorderBefore(const Box& layoutBox)
+{
+ return hasBorder(layoutBox.style().borderBefore());
+}
+
+static bool hasBorderAfter(const Box& layoutBox)
+{
+ return hasBorder(layoutBox.style().borderAfter());
+}
+
+static bool hasPaddingBefore(const Box& layoutBox)
+{
+ return hasPadding(layoutBox.style().paddingBefore());
+}
+
+static bool hasPaddingAfter(const Box& layoutBox)
+{
+ return hasPadding(layoutBox.style().paddingAfter());
+}
+
static LayoutUnit marginValue(LayoutUnit currentMarginValue, LayoutUnit candidateMarginValue)
{
if (!candidateMarginValue)
@@ -94,13 +127,13 @@
if (layoutBox.isFloatingOrOutOfFlowPositioned())
return false;
- // We never margin collapse the initial containing block.
- ASSERT(layoutBox.parent());
- auto& parent = *layoutBox.parent();
// Only the first inlflow child collapses with parent.
if (layoutBox.previousInFlowSibling())
return false;
+ // We never margin collapse the initial containing block.
+ ASSERT(layoutBox.parent());
+ auto& parent = *layoutBox.parent();
if (parent.establishesBlockFormattingContext())
return false;
@@ -108,11 +141,10 @@
if (parent.isDocumentBox() || parent.isInitialContainingBlock())
return false;
- auto& parentDisplayBox = layoutState.displayBoxForLayoutBox(parent);
- if (parentDisplayBox.borderTop())
+ if (hasBorderBefore(parent))
return false;
- if (parentDisplayBox.paddingTop().value_or(0))
+ if (hasPaddingBefore(parent))
return false;
if (BlockFormattingContext::Quirks::shouldIgnoreMarginTop(layoutState, layoutBox))
@@ -121,17 +153,15 @@
return true;
}
-static bool isMarginBottomCollapsedThrough(const LayoutState& layoutState, const Box& layoutBox)
+static bool isMarginBottomCollapsedThrough(const Box& layoutBox)
{
ASSERT(layoutBox.isBlockLevelBox());
// If the top and bottom margins of a box are adjoining, then it is possible for margins to collapse through it.
- auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
-
- if (displayBox.borderTop() || displayBox.borderBottom())
+ if (hasBorderBefore(layoutBox) || hasBorderAfter(layoutBox))
return false;
- if (displayBox.paddingTop().value_or(0) || displayBox.paddingBottom().value_or(0))
+ if (hasPaddingBefore(layoutBox) || hasPaddingAfter(layoutBox))
return false;
if (!layoutBox.style().height().isAuto() || !layoutBox.style().minHeight().isAuto())
@@ -219,7 +249,7 @@
return 0;
if (!isMarginTopCollapsedWithSibling(layoutBox)) {
- if (!isMarginBottomCollapsedThrough(layoutState, layoutBox))
+ if (!isMarginBottomCollapsedThrough(layoutBox))
return nonCollapsedMarginTop(layoutState, layoutBox);
// Compute the collapsed through value.
auto marginTop = nonCollapsedMarginTop(layoutState, layoutBox);
@@ -246,10 +276,10 @@
ASSERT(layoutBox.isBlockLevelBox());
// TODO: take _hasAdjoiningMarginTopAndBottom() into account.
- if (isMarginBottomCollapsedWithParent(layoutState, layoutBox))
+ if (isMarginBottomCollapsedWithParent(layoutBox))
return 0;
- if (isMarginBottomCollapsedThrough(layoutState, layoutBox))
+ if (isMarginBottomCollapsedThrough(layoutBox))
return 0;
// Floats and out of flow positioned boxes do not collapse their margins.
@@ -263,7 +293,7 @@
return nonCollapsedMarginBottom(layoutState, layoutBox);
}
-bool BlockFormattingContext::Geometry::MarginCollapse::isMarginBottomCollapsedWithParent(const LayoutState& layoutState, const Box& layoutBox)
+bool BlockFormattingContext::Geometry::MarginCollapse::isMarginBottomCollapsedWithParent(const Box& layoutBox)
{
// last inflow box to parent.
// https://www.w3.org/TR/CSS21/box.html#collapsing-margins
@@ -275,7 +305,7 @@
if (layoutBox.isFloatingOrOutOfFlowPositioned())
return false;
- if (isMarginBottomCollapsedThrough(layoutState, layoutBox))
+ if (isMarginBottomCollapsedThrough(layoutBox))
return false;
// We never margin collapse the initial containing block.
@@ -292,11 +322,10 @@
if (parent.isDocumentBox() || parent.isInitialContainingBlock())
return false;
- auto& parentDisplayBox = layoutState.displayBoxForLayoutBox(parent);
- if (parentDisplayBox.borderTop())
+ if (hasBorderBefore(parent))
return false;
- if (parentDisplayBox.paddingTop().value_or(0))
+ if (hasPaddingBefore(parent))
return false;
if (!parent.style().height().isAuto())
@@ -324,7 +353,7 @@
// FIXME: Check for collapsed through margin.
auto& lastInFlowChild = *downcast<Container>(layoutBox).lastInFlowChild();
- if (!isMarginBottomCollapsedWithParent(layoutState, lastInFlowChild))
+ if (!isMarginBottomCollapsedWithParent(lastInFlowChild))
return 0;
// Collect collapsed margin bottom recursively.
Modified: trunk/Tools/ChangeLog (238734 => 238735)
--- trunk/Tools/ChangeLog 2018-11-30 14:52:43 UTC (rev 238734)
+++ trunk/Tools/ChangeLog 2018-11-30 15:22:39 UTC (rev 238735)
@@ -1,3 +1,12 @@
+2018-11-30 Zalan Bujtas <[email protected]>
+
+ [LFC][BFC][MarginCollapsing] Do not use computed display box values for border and padding
+ https://bugs.webkit.org/show_bug.cgi?id=192214
+
+ Reviewed by Antti Koivisto.
+
+ * LayoutReloaded/misc/LFC-passing-tests.txt:
+
2018-11-30 Thibault Saunier <[email protected]>
[GTK][MiniBrowser] Handle Device Info permission requests
Modified: trunk/Tools/LayoutReloaded/misc/LFC-passing-tests.txt (238734 => 238735)
--- trunk/Tools/LayoutReloaded/misc/LFC-passing-tests.txt 2018-11-30 14:52:43 UTC (rev 238734)
+++ trunk/Tools/LayoutReloaded/misc/LFC-passing-tests.txt 2018-11-30 15:22:39 UTC (rev 238735)
@@ -140,3 +140,4 @@
fast/block/positioning/049.html
fast/block/positioning/052.html
fast/block/positioning/054.html
+fast/block/inside-inlines/basic-float-intrusion.html
