Modified: trunk/Source/WebCore/ChangeLog (234082 => 234083)
--- trunk/Source/WebCore/ChangeLog 2018-07-21 13:10:43 UTC (rev 234082)
+++ trunk/Source/WebCore/ChangeLog 2018-07-21 15:00:30 UTC (rev 234083)
@@ -1,3 +1,31 @@
+2018-07-21 Zalan Bujtas <[email protected]>
+
+ [LFC][BFC] Do not collapse top/bottom margin with first/last inflow child from a non-block formatting context.
+ https://bugs.webkit.org/show_bug.cgi?id=187867
+
+ Reviewed by Antti Koivisto.
+
+ The box's top/bottom margin never collapses with a non-block inflow child.
+
+ * layout/blockformatting/BlockMarginCollapse.cpp:
+ (WebCore::Layout::isMarginTopCollapsedWithSibling):
+ (WebCore::Layout::isMarginBottomCollapsedWithSibling):
+ (WebCore::Layout::isMarginTopCollapsedWithParent):
+ (WebCore::Layout::isMarginBottomCollapsedThrough):
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::collapsedMarginTopFromFirstChild):
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::nonCollapsedMarginTop):
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::computedNonCollapsedMarginTop):
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::computedNonCollapsedMarginBottom):
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginTop):
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginBottom):
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::isMarginBottomCollapsedWithParent):
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::collapsedMarginBottomFromLastChild):
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::nonCollapsedMarginBottom):
+ * layout/layouttree/LayoutBox.cpp:
+ (WebCore::Layout::Box::establishesBlockFormattingContextOnly const): <div style="overflow: hidden">foobar</div> establishes both inline and block formatting context (inline wins though).
+ * layout/layouttree/LayoutBox.h: establishesBlockFormattingContext() does not need to be virtual since we can determine it by looking at the box's style. -while in case
+ of inline formatting context, it is about the content.
+
2018-07-20 Jer Noble <[email protected]>
REGRESSION (r233974): Cannot close pip'd video; pops back into PiP.
Modified: trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp (234082 => 234083)
--- trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp 2018-07-21 13:10:43 UTC (rev 234082)
+++ trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp 2018-07-21 15:00:30 UTC (rev 234083)
@@ -54,6 +54,8 @@
static bool isMarginTopCollapsedWithSibling(const Box& layoutBox)
{
+ ASSERT(layoutBox.isBlockLevelBox());
+
if (layoutBox.isFloatingPositioned())
return false;
@@ -67,6 +69,8 @@
static bool isMarginBottomCollapsedWithSibling(const Box& layoutBox)
{
+ ASSERT(layoutBox.isBlockLevelBox());
+
if (layoutBox.isFloatingPositioned())
return false;
@@ -85,6 +89,8 @@
if (layoutBox.isAnonymous())
return false;
+ ASSERT(layoutBox.isBlockLevelBox());
+
if (layoutBox.isFloatingOrOutOfFlowPositioned())
return false;
@@ -114,6 +120,8 @@
static bool isMarginBottomCollapsedThrough(const LayoutContext& layoutContext, 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 = *layoutContext.displayBoxForLayoutBox(layoutBox);
@@ -138,10 +146,16 @@
LayoutUnit BlockFormattingContext::MarginCollapse::collapsedMarginTopFromFirstChild(const LayoutContext& layoutContext, const Box& layoutBox)
{
+ ASSERT(layoutBox.isBlockLevelBox());
+
// Check if the first child collapses its margin top.
if (!is<Container>(layoutBox) || !downcast<Container>(layoutBox).hasInFlowChild())
return 0;
+ // Do not collapse margin with a box from a non-block formatting context <div><span>foobar</span></div>.
+ if (layoutBox.establishesFormattingContext() && !layoutBox.establishesBlockFormattingContextOnly())
+ return 0;
+
// FIXME: Take collapsed through margin into account.
auto& firstInFlowChild = *downcast<Container>(layoutBox).firstInFlowChild();
if (!isMarginTopCollapsedWithParent(layoutContext, firstInFlowChild))
@@ -152,6 +166,8 @@
LayoutUnit BlockFormattingContext::MarginCollapse::nonCollapsedMarginTop(const LayoutContext& layoutContext, const Box& layoutBox)
{
+ ASSERT(layoutBox.isBlockLevelBox());
+
// Non collapsed margin top includes collapsed margin from inflow first child.
return marginValue(computedNonCollapsedMarginTop(layoutContext, layoutBox), collapsedMarginTopFromFirstChild(layoutContext, layoutBox));
}
@@ -172,11 +188,15 @@
}*/
LayoutUnit BlockFormattingContext::MarginCollapse::computedNonCollapsedMarginTop(const LayoutContext& layoutContext, const Box& layoutBox)
{
+ ASSERT(layoutBox.isBlockLevelBox());
+
return FormattingContext::Geometry::computedNonCollapsedVerticalMarginValue(layoutContext, layoutBox).top;
}
LayoutUnit BlockFormattingContext::MarginCollapse::computedNonCollapsedMarginBottom(const LayoutContext& layoutContext, const Box& layoutBox)
{
+ ASSERT(layoutBox.isBlockLevelBox());
+
return FormattingContext::Geometry::computedNonCollapsedVerticalMarginValue(layoutContext, layoutBox).bottom;
}
@@ -185,6 +205,8 @@
if (layoutBox.isAnonymous())
return 0;
+ ASSERT(layoutBox.isBlockLevelBox());
+
// TODO: take _hasAdjoiningMarginTopAndBottom() into account.
if (isMarginTopCollapsedWithParent(layoutContext, layoutBox))
return 0;
@@ -214,6 +236,8 @@
if (layoutBox.isAnonymous())
return 0;
+ ASSERT(layoutBox.isBlockLevelBox());
+
// TODO: take _hasAdjoiningMarginTopAndBottom() into account.
if (isMarginBottomCollapsedWithParent(layoutContext, layoutBox))
return 0;
@@ -239,6 +263,8 @@
if (layoutBox.isAnonymous())
return false;
+ ASSERT(layoutBox.isBlockLevelBox());
+
if (layoutBox.isFloatingOrOutOfFlowPositioned())
return false;
@@ -279,10 +305,16 @@
LayoutUnit BlockFormattingContext::MarginCollapse::collapsedMarginBottomFromLastChild(const LayoutContext& layoutContext, const Box& layoutBox)
{
+ ASSERT(layoutBox.isBlockLevelBox());
+
// Check if the last child propagates its margin bottom.
if (!is<Container>(layoutBox) || !downcast<Container>(layoutBox).hasInFlowChild())
return 0;
+ // Do not collapse margin with a box from a non-block formatting context <div><span>foobar</span></div>.
+ if (layoutBox.establishesFormattingContext() && !layoutBox.establishesBlockFormattingContextOnly())
+ return 0;
+
// FIXME: Check for collapsed through margin.
auto& lastInFlowChild = *downcast<Container>(layoutBox).lastInFlowChild();
if (!isMarginBottomCollapsedWithParent(layoutContext, lastInFlowChild))
@@ -294,6 +326,8 @@
LayoutUnit BlockFormattingContext::MarginCollapse::nonCollapsedMarginBottom(const LayoutContext& layoutContext, const Box& layoutBox)
{
+ ASSERT(layoutBox.isBlockLevelBox());
+
// Non collapsed margin bottom includes collapsed margin from inflow last child.
return marginValue(computedNonCollapsedMarginBottom(layoutContext, layoutBox), collapsedMarginBottomFromLastChild(layoutContext, layoutBox));
}
Modified: trunk/Source/WebCore/layout/layouttree/LayoutBox.cpp (234082 => 234083)
--- trunk/Source/WebCore/layout/layouttree/LayoutBox.cpp 2018-07-21 13:10:43 UTC (rev 234082)
+++ trunk/Source/WebCore/layout/layouttree/LayoutBox.cpp 2018-07-21 15:00:30 UTC (rev 234083)
@@ -70,6 +70,11 @@
return false;
}
+bool Box::establishesBlockFormattingContextOnly() const
+{
+ return establishesBlockFormattingContext() && !establishesInlineFormattingContext();
+}
+
bool Box::isRelativelyPositioned() const
{
return m_style.position() == PositionType::Relative;
Modified: trunk/Source/WebCore/layout/layouttree/LayoutBox.h (234082 => 234083)
--- trunk/Source/WebCore/layout/layouttree/LayoutBox.h 2018-07-21 13:10:43 UTC (rev 234082)
+++ trunk/Source/WebCore/layout/layouttree/LayoutBox.h 2018-07-21 15:00:30 UTC (rev 234083)
@@ -47,7 +47,8 @@
virtual ~Box();
bool establishesFormattingContext() const;
- virtual bool establishesBlockFormattingContext() const;
+ bool establishesBlockFormattingContext() const;
+ bool establishesBlockFormattingContextOnly() const;
virtual bool establishesInlineFormattingContext() const { return false; }
bool isInFlow() const { return !isFloatingOrOutOfFlowPositioned(); }
