Diff
Modified: trunk/Source/WebCore/ChangeLog (239674 => 239675)
--- trunk/Source/WebCore/ChangeLog 2019-01-07 16:16:42 UTC (rev 239674)
+++ trunk/Source/WebCore/ChangeLog 2019-01-07 16:18:52 UTC (rev 239675)
@@ -1,5 +1,31 @@
2019-01-07 Zalan Bujtas <[email protected]>
+ [LFC][BFC] Margin collapsing should not be limited to in-flow non-replaced boxes.
+ https://bugs.webkit.org/show_bug.cgi?id=193183
+
+ Reviewed by Antti Koivisto.
+
+ * layout/FormattingContext.cpp:
+ (WebCore::Layout::FormattingContext::computeOutOfFlowVerticalGeometry const):
+ * layout/FormattingContextGeometry.cpp:
+ (WebCore::Layout::FormattingContext::Geometry::outOfFlowNonReplacedVerticalGeometry):
+ (WebCore::Layout::FormattingContext::Geometry::outOfFlowReplacedVerticalGeometry):
+ (WebCore::Layout::FormattingContext::Geometry::complicatedCases):
+ (WebCore::Layout::FormattingContext::Geometry::floatingNonReplacedWidthAndMargin):
+ (WebCore::Layout::FormattingContext::Geometry::inlineReplacedHeightAndMargin):
+ * layout/LayoutUnits.h:
+ * layout/blockformatting/BlockFormattingContext.cpp:
+ (WebCore::Layout::BlockFormattingContext::computeHeightAndMargin const):
+ * layout/blockformatting/BlockFormattingContextGeometry.cpp:
+ (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMargin):
+ (WebCore::Layout::BlockFormattingContext::Geometry::inFlowHeightAndMargin):
+ * layout/blockformatting/BlockFormattingContextQuirks.cpp:
+ (WebCore::Layout::BlockFormattingContext::Quirks::stretchedHeight):
+ * layout/inlineformatting/InlineFormattingContext.cpp:
+ (WebCore::Layout::InlineFormattingContext::computeHeightAndMargin const):
+
+2019-01-07 Zalan Bujtas <[email protected]>
+
[LFC][BFC] Move MarginCollapse from BlockFormattingContext::Geometry to BlockFormattingContext
https://bugs.webkit.org/show_bug.cgi?id=193181
Modified: trunk/Source/WebCore/layout/FormattingContext.cpp (239674 => 239675)
--- trunk/Source/WebCore/layout/FormattingContext.cpp 2019-01-07 16:16:42 UTC (rev 239674)
+++ trunk/Source/WebCore/layout/FormattingContext.cpp 2019-01-07 16:18:52 UTC (rev 239675)
@@ -115,12 +115,11 @@
}
auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
- // Margins of absolutely positioned boxes do not collapse
- ASSERT(!verticalGeometry.heightAndMargin.usedMargin.hasCollapsedValues());
- auto nonCollapsedVerticalMargin = verticalGeometry.heightAndMargin.usedMargin.nonCollapsedValues();
+ auto nonCollapsedVerticalMargin = verticalGeometry.heightAndMargin.nonCollapsedMargin;
displayBox.setTop(verticalGeometry.top + nonCollapsedVerticalMargin.before);
displayBox.setContentBoxHeight(verticalGeometry.heightAndMargin.height);
- displayBox.setVerticalMargin(verticalGeometry.heightAndMargin.usedMargin);
+ // Margins of absolutely positioned boxes do not collapse
+ displayBox.setVerticalMargin({ nonCollapsedVerticalMargin, { } });
}
void FormattingContext::computeBorderAndPadding(const Box& layoutBox) const
Modified: trunk/Source/WebCore/layout/FormattingContextGeometry.cpp (239674 => 239675)
--- trunk/Source/WebCore/layout/FormattingContextGeometry.cpp 2019-01-07 16:16:42 UTC (rev 239674)
+++ trunk/Source/WebCore/layout/FormattingContextGeometry.cpp 2019-01-07 16:18:52 UTC (rev 239675)
@@ -351,7 +351,7 @@
ASSERT(height);
LOG_WITH_STREAM(FormattingContextLayout, stream << "[Position][Height][Margin] -> out-of-flow non-replaced -> top(" << *top << "px) bottom(" << *bottom << "px) height(" << *height << "px) margin(" << usedVerticalMargin.before << "px, " << usedVerticalMargin.after << "px) layoutBox(" << &layoutBox << ")");
- return { *top, *bottom, { *height, { usedVerticalMargin, { } } } };
+ return { *top, *bottom, { *height, usedVerticalMargin } };
}
HorizontalGeometry FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGeometry(LayoutState& layoutState, const Box& layoutBox, Optional<LayoutUnit> usedWidth)
@@ -558,7 +558,7 @@
bottom = containingBlockHeight - (*top + usedVerticalMargin.before + borderTop + paddingTop + height + paddingBottom + borderBottom + usedVerticalMargin.after);
LOG_WITH_STREAM(FormattingContextLayout, stream << "[Position][Height][Margin] -> out-of-flow replaced -> top(" << *top << "px) bottom(" << *bottom << "px) height(" << height << "px) margin(" << usedVerticalMargin.before << "px, " << usedVerticalMargin.after << "px) layoutBox(" << &layoutBox << ")");
- return { *top, *bottom, { height, { usedVerticalMargin, { } } } };
+ return { *top, *bottom, { height, usedVerticalMargin } };
}
HorizontalGeometry FormattingContext::Geometry::outOfFlowReplacedHorizontalGeometry(const LayoutState& layoutState, const Box& layoutBox, Optional<LayoutUnit> usedWidth)
@@ -682,7 +682,7 @@
ASSERT(height);
LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> floating non-replaced -> height(" << *height << "px) margin(" << usedVerticalMargin.before << "px, " << usedVerticalMargin.after << "px) -> layoutBox(" << &layoutBox << ")");
- return HeightAndMargin { *height, { usedVerticalMargin, { } } };
+ return HeightAndMargin { *height, usedVerticalMargin };
}
WidthAndMargin FormattingContext::Geometry::floatingNonReplacedWidthAndMargin(LayoutState& layoutState, const Box& layoutBox, Optional<LayoutUnit> usedWidth)
@@ -696,16 +696,17 @@
auto& containingBlock = *layoutBox.containingBlock();
auto containingBlockWidth = layoutState.displayBoxForLayoutBox(containingBlock).contentBoxWidth();
+ auto computedHorizontalMargin = Geometry::computedHorizontalMargin(layoutState, layoutBox);
// #1
- auto computedHorizontalMargin = Geometry::computedHorizontalMargin(layoutState, layoutBox);
+ auto usedHorizontallMargin = UsedHorizontalMargin { computedHorizontalMargin.start.valueOr(0), computedHorizontalMargin.end.valueOr(0) };
// #2
auto width = computedValueIfNotAuto(usedWidth ? Length { usedWidth.value(), Fixed } : layoutBox.style().logicalWidth(), containingBlockWidth);
if (!width)
width = shrinkToFitWidth(layoutState, layoutBox);
- LOG_WITH_STREAM(FormattingContextLayout, stream << "[Width][Margin] -> floating non-replaced -> width(" << *width << "px) margin(" << computedHorizontalMargin.start.valueOr(0_lu) << "px, " << computedHorizontalMargin.end.valueOr(0_lu) << "px) -> layoutBox(" << &layoutBox << ")");
- return WidthAndMargin { *width, { computedHorizontalMargin.start.valueOr(0_lu), computedHorizontalMargin.end.valueOr(0_lu) }, computedHorizontalMargin };
+ LOG_WITH_STREAM(FormattingContextLayout, stream << "[Width][Margin] -> floating non-replaced -> width(" << *width << "px) margin(" << usedHorizontallMargin.start << "px, " << usedHorizontallMargin.end << "px) -> layoutBox(" << &layoutBox << ")");
+ return WidthAndMargin { *width, usedHorizontallMargin, computedHorizontalMargin };
}
HeightAndMargin FormattingContext::Geometry::floatingReplacedHeightAndMargin(const LayoutState& layoutState, const Box& layoutBox, Optional<LayoutUnit> usedHeight)
@@ -810,7 +811,7 @@
ASSERT(height);
LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow replaced -> height(" << *height << "px) margin(" << usedVerticalMargin.before << "px, " << usedVerticalMargin.after << "px) -> layoutBox(" << &layoutBox << ")");
- return { *height, { usedVerticalMargin, { } } };
+ return { *height, usedVerticalMargin };
}
WidthAndMargin FormattingContext::Geometry::inlineReplacedWidthAndMargin(const LayoutState& layoutState, const Box& layoutBox,
Modified: trunk/Source/WebCore/layout/LayoutUnits.h (239674 => 239675)
--- trunk/Source/WebCore/layout/LayoutUnits.h 2019-01-07 16:16:42 UTC (rev 239674)
+++ trunk/Source/WebCore/layout/LayoutUnits.h 2019-01-07 16:18:52 UTC (rev 239675)
@@ -109,7 +109,7 @@
struct HeightAndMargin {
LayoutUnit height;
- UsedVerticalMargin usedMargin;
+ UsedVerticalMargin::NonCollapsedValues nonCollapsedMargin;
};
struct HorizontalGeometry {
Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp (239674 => 239675)
--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp 2019-01-07 16:16:42 UTC (rev 239674)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp 2019-01-07 16:18:52 UTC (rev 239675)
@@ -358,7 +358,7 @@
auto maxHeightAndMargin = compute(maxHeight);
// Used height should remain the same.
ASSERT((layoutState.inQuirksMode() && (layoutBox.isBodyBox() || layoutBox.isDocumentBox())) || maxHeightAndMargin.height == *maxHeight);
- heightAndMargin = { *maxHeight, maxHeightAndMargin.usedMargin };
+ heightAndMargin = { *maxHeight, maxHeightAndMargin.nonCollapsedMargin };
}
}
@@ -367,17 +367,19 @@
auto minHeightAndMargin = compute(minHeight);
// Used height should remain the same.
ASSERT((layoutState.inQuirksMode() && (layoutBox.isBodyBox() || layoutBox.isDocumentBox())) || minHeightAndMargin.height == *minHeight);
- heightAndMargin = { *minHeight, minHeightAndMargin.usedMargin };
+ heightAndMargin = { *minHeight, minHeightAndMargin.nonCollapsedMargin };
}
}
+ auto collapsedMargin = UsedVerticalMargin::CollapsedValues { MarginCollapse::marginBefore(layoutState, layoutBox), MarginCollapse::marginAfter(layoutState, layoutBox) };
+ auto verticalMargin = UsedVerticalMargin { heightAndMargin.nonCollapsedMargin, collapsedMargin };
auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
displayBox.setContentBoxHeight(heightAndMargin.height);
- displayBox.setVerticalMargin(heightAndMargin.usedMargin);
+ displayBox.setVerticalMargin(verticalMargin);
// If this box has already been moved by the estimated vertical margin, no need to move it again.
if (layoutBox.isFloatingPositioned() || !displayBox.estimatedMarginBefore())
- displayBox.moveVertically(heightAndMargin.usedMargin.before());
+ displayBox.moveVertically(verticalMargin.before());
}
FormattingContext::InstrinsicWidthConstraints BlockFormattingContext::instrinsicWidthConstraints() const
Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp (239674 => 239675)
--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp 2019-01-07 16:16:42 UTC (rev 239674)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp 2019-01-07 16:18:52 UTC (rev 239675)
@@ -60,15 +60,14 @@
auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
auto computedVerticalMargin = Geometry::computedVerticalMargin(layoutState, layoutBox);
auto nonCollapsedMargin = UsedVerticalMargin::NonCollapsedValues { computedVerticalMargin.before.valueOr(0), computedVerticalMargin.after.valueOr(0) };
- auto collapsedMargin = UsedVerticalMargin::CollapsedValues { MarginCollapse::marginBefore(layoutState, layoutBox), MarginCollapse::marginAfter(layoutState, layoutBox) };
auto borderAndPaddingTop = displayBox.borderTop() + displayBox.paddingTop().valueOr(0);
auto height = usedHeight ? usedHeight.value() : computedHeightValue(layoutState, layoutBox, HeightType::Normal);
if (height)
- return { height.value(), { nonCollapsedMargin, collapsedMargin } };
+ return { height.value(), nonCollapsedMargin };
if (!is<Container>(layoutBox) || !downcast<Container>(layoutBox).hasInFlowChild())
- return { 0, { nonCollapsedMargin, collapsedMargin } };
+ return { 0, nonCollapsedMargin };
// 1. the bottom edge of the last line box, if the box establishes a inline formatting context with one or more lines
if (layoutBox.establishesInlineFormattingContext()) {
@@ -75,7 +74,7 @@
// This is temp and will be replaced by the correct display box once inline runs move over to the display tree.
auto& inlineRuns = downcast<InlineFormattingState>(layoutState.establishedFormattingState(layoutBox)).inlineRuns();
auto bottomEdge = inlineRuns.isEmpty() ? LayoutUnit() : inlineRuns.last().logicalBottom();
- return { bottomEdge, { nonCollapsedMargin, collapsedMargin } };
+ return { bottomEdge, nonCollapsedMargin };
}
// 2. the bottom edge of the bottom (possibly collapsed) margin of its last in-flow child, if the child's bottom margin...
@@ -83,7 +82,7 @@
ASSERT(lastInFlowChild);
if (!MarginCollapse::marginAfterCollapsesWithParentMarginAfter(layoutState, *lastInFlowChild)) {
auto& lastInFlowDisplayBox = layoutState.displayBoxForLayoutBox(*lastInFlowChild);
- return { lastInFlowDisplayBox.bottom() + lastInFlowDisplayBox.marginAfter() - borderAndPaddingTop, { nonCollapsedMargin, collapsedMargin } };
+ return { lastInFlowDisplayBox.bottom() + lastInFlowDisplayBox.marginAfter() - borderAndPaddingTop, nonCollapsedMargin };
}
// 3. the bottom border edge of the last in-flow child whose top margin doesn't collapse with the element's bottom margin
@@ -92,16 +91,16 @@
inFlowChild = inFlowChild->previousInFlowSibling();
if (inFlowChild) {
auto& inFlowDisplayBox = layoutState.displayBoxForLayoutBox(*inFlowChild);
- return { inFlowDisplayBox.top() + inFlowDisplayBox.borderBox().height() - borderAndPaddingTop, { nonCollapsedMargin, collapsedMargin } };
+ return { inFlowDisplayBox.top() + inFlowDisplayBox.borderBox().height() - borderAndPaddingTop, nonCollapsedMargin };
}
// 4. zero, otherwise
- return { 0, { nonCollapsedMargin, collapsedMargin } };
+ return { 0, nonCollapsedMargin };
};
auto heightAndMargin = compute();
- LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> height(" << heightAndMargin.height << "px) margin(" << heightAndMargin.usedMargin.before() << "px, " << heightAndMargin.usedMargin.after() << "px) -> layoutBox(" << &layoutBox << ")");
+ LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> height(" << heightAndMargin.height << "px) margin(" << heightAndMargin.nonCollapsedMargin.before << "px, " << heightAndMargin.nonCollapsedMargin.after << "px) -> layoutBox(" << &layoutBox << ")");
return heightAndMargin;
}
@@ -260,7 +259,7 @@
heightAndMargin = Quirks::stretchedHeight(layoutState, layoutBox, heightAndMargin);
- LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> streched to viewport -> height(" << heightAndMargin.height << "px) margin(" << heightAndMargin.usedMargin.before() << "px, " << heightAndMargin.usedMargin.after() << "px) -> layoutBox(" << &layoutBox << ")");
+ LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> streched to viewport -> height(" << heightAndMargin.height << "px) margin(" << heightAndMargin.nonCollapsedMargin.before << "px, " << heightAndMargin.nonCollapsedMargin.after << "px) -> layoutBox(" << &layoutBox << ")");
return heightAndMargin;
}
Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp (239674 => 239675)
--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp 2019-01-07 16:16:42 UTC (rev 239674)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp 2019-01-07 16:18:52 UTC (rev 239675)
@@ -80,10 +80,9 @@
LayoutUnit totalVerticalMargin;
if (layoutBox.isDocumentBox()) {
- auto verticalMargin = heightAndMargin.usedMargin;
// Document box's margins do not collapse.
- ASSERT(!verticalMargin.hasCollapsedValues());
- totalVerticalMargin = verticalMargin.before() + verticalMargin.after();
+ auto verticalMargin = heightAndMargin.nonCollapsedMargin;
+ totalVerticalMargin = verticalMargin.before + verticalMargin.after;
} else if (layoutBox.isBodyBox()) {
// Here is the quirky part for body box:
// Stretch the body using the initial containing block's height and shrink it with document box's margin/border/padding.
@@ -91,14 +90,8 @@
auto verticalMargin = Geometry::estimatedMarginBefore(layoutState, documentBox) + Geometry::estimatedMarginAfter(layoutState, documentBox);
strechedHeight -= verticalMargin;
- // This quirk happens when the body height is 0 which means its vertical margins collapse through (top and bottom margins are adjoining).
- // However now that we stretch the body they don't collapse through anymore, so we need to use the non-collapsed values instead.
- if (heightAndMargin.height)
- totalVerticalMargin = heightAndMargin.usedMargin.before() + heightAndMargin.usedMargin.after();
- else {
- auto nonCollapsedValues = heightAndMargin.usedMargin.nonCollapsedValues();
- totalVerticalMargin = nonCollapsedValues.before + nonCollapsedValues.after;
- }
+ auto nonCollapsedValues = heightAndMargin.nonCollapsedMargin;
+ totalVerticalMargin = nonCollapsedValues.before + nonCollapsedValues.after;
}
// Stretch but never overstretch with the margins.
Modified: trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp (239674 => 239675)
--- trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp 2019-01-07 16:16:42 UTC (rev 239674)
+++ trunk/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp 2019-01-07 16:18:52 UTC (rev 239675)
@@ -362,7 +362,7 @@
auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
displayBox.setContentBoxHeight(heightAndMargin.height);
- displayBox.setVerticalMargin(heightAndMargin.usedMargin);
+ displayBox.setVerticalMargin({ heightAndMargin.nonCollapsedMargin, { } });
}
void InlineFormattingContext::layoutFormattingContextRoot(const Box& root) const
