- Revision
- 240583
- Author
- [email protected]
- Date
- 2019-01-28 08:20:17 -0800 (Mon, 28 Jan 2019)
Log Message
[LFC][MarginCollapsing][Quirks] Quirk margin values get propagated through margin collapsing
https://bugs.webkit.org/show_bug.cgi?id=193896
Reviewed by Antti Koivisto.
Source/WebCore:
This patch implements quirk margin value collapsing. There are a few "quirk" rules when it comes to margin collapsing.
1. Collapsed quirk margin values are ignored on quirk containers
<body>
<p> p elements have 1em vertical (top/bottom) quirk margin
</body>
In quirk mode, <p> and <body> (quirk container) collapses their vertical margins but <p>'s quirk values(1qem -> 16px) are ignored.
Used vertical margin values on the <body> are top: 8px bottom: 8px.
2. Quirk margin values are turned into non-quirk values when collapsed with non-zero, non-quirk margins.
<body>
<div style="margin-top: 1px">
<p> p elements have 1em vertical (top/bottom) quirk margin
</div>
</body>
When <p>'s vertical margin collapses with the parent <div>,
- the collapsed before value becomes 16px (max(1qem, 1px)) and this collapsed value is now considered as a non-quirk value
- the collapsed after value stays 1qem quirk value.
When <div> collapses with <body>
- the collapsed before value becomes 16px (max(16px, 8px))
- the <div>'s quirk after value gets ignored and the collapsed after value stays 8px.
Used vertical margin values on the <body> are top: 16px (1em) bottom: 8px.
* layout/MarginTypes.h:
(WebCore::Layout::PositiveAndNegativeVerticalMargin::Values::isNonZero const):
* layout/blockformatting/BlockFormattingContext.h:
* layout/blockformatting/BlockFormattingContextQuirks.cpp:
(WebCore::Layout::BlockFormattingContext::Quirks::shouldIgnoreCollapsedQuirkMargin):
(WebCore::Layout::hasMarginBeforeQuirkValue): Deleted.
(WebCore::Layout::BlockFormattingContext::Quirks::shouldIgnoreMarginBefore): Deleted.
(WebCore::Layout::BlockFormattingContext::Quirks::shouldIgnoreMarginAfter): Deleted.
* layout/blockformatting/BlockMarginCollapse.cpp:
(WebCore::Layout::BlockFormattingContext::MarginCollapse::marginBeforeCollapsesWithPreviousSiblingMarginAfter):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::marginBeforeCollapsesWithFirstInFlowChildMarginBefore):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::marginAfterCollapsesWithLastInFlowChildMarginAfter):
(WebCore::Layout::computedPositiveAndNegativeMargin):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::positiveNegativeMarginBefore):
(WebCore::Layout::BlockFormattingContext::MarginCollapse::positiveNegativeMarginAfter):
Tools:
* LayoutReloaded/misc/LFC-passing-tests.txt:
Modified Paths
Diff
Modified: trunk/Source/WebCore/ChangeLog (240582 => 240583)
--- trunk/Source/WebCore/ChangeLog 2019-01-28 15:43:00 UTC (rev 240582)
+++ trunk/Source/WebCore/ChangeLog 2019-01-28 16:20:17 UTC (rev 240583)
@@ -1,5 +1,56 @@
2019-01-28 Zalan Bujtas <[email protected]>
+ [LFC][MarginCollapsing][Quirks] Quirk margin values get propagated through margin collapsing
+ https://bugs.webkit.org/show_bug.cgi?id=193896
+
+ Reviewed by Antti Koivisto.
+
+ This patch implements quirk margin value collapsing. There are a few "quirk" rules when it comes to margin collapsing.
+
+ 1. Collapsed quirk margin values are ignored on quirk containers
+
+ <body>
+ <p> p elements have 1em vertical (top/bottom) quirk margin
+ </body>
+
+ In quirk mode, <p> and <body> (quirk container) collapses their vertical margins but <p>'s quirk values(1qem -> 16px) are ignored.
+ Used vertical margin values on the <body> are top: 8px bottom: 8px.
+
+ 2. Quirk margin values are turned into non-quirk values when collapsed with non-zero, non-quirk margins.
+
+ <body>
+ <div style="margin-top: 1px">
+ <p> p elements have 1em vertical (top/bottom) quirk margin
+ </div>
+ </body>
+
+ When <p>'s vertical margin collapses with the parent <div>,
+ - the collapsed before value becomes 16px (max(1qem, 1px)) and this collapsed value is now considered as a non-quirk value
+ - the collapsed after value stays 1qem quirk value.
+
+ When <div> collapses with <body>
+ - the collapsed before value becomes 16px (max(16px, 8px))
+ - the <div>'s quirk after value gets ignored and the collapsed after value stays 8px.
+ Used vertical margin values on the <body> are top: 16px (1em) bottom: 8px.
+
+ * layout/MarginTypes.h:
+ (WebCore::Layout::PositiveAndNegativeVerticalMargin::Values::isNonZero const):
+ * layout/blockformatting/BlockFormattingContext.h:
+ * layout/blockformatting/BlockFormattingContextQuirks.cpp:
+ (WebCore::Layout::BlockFormattingContext::Quirks::shouldIgnoreCollapsedQuirkMargin):
+ (WebCore::Layout::hasMarginBeforeQuirkValue): Deleted.
+ (WebCore::Layout::BlockFormattingContext::Quirks::shouldIgnoreMarginBefore): Deleted.
+ (WebCore::Layout::BlockFormattingContext::Quirks::shouldIgnoreMarginAfter): Deleted.
+ * layout/blockformatting/BlockMarginCollapse.cpp:
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginBeforeCollapsesWithPreviousSiblingMarginAfter):
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginBeforeCollapsesWithFirstInFlowChildMarginBefore):
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::marginAfterCollapsesWithLastInFlowChildMarginAfter):
+ (WebCore::Layout::computedPositiveAndNegativeMargin):
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::positiveNegativeMarginBefore):
+ (WebCore::Layout::BlockFormattingContext::MarginCollapse::positiveNegativeMarginAfter):
+
+2019-01-28 Zalan Bujtas <[email protected]>
+
[LFC][BFC] Remove redundant vertical positioning in BlockFormattingContext::computeFloatingPosition
https://bugs.webkit.org/show_bug.cgi?id=193872
Modified: trunk/Source/WebCore/layout/MarginTypes.h (240582 => 240583)
--- trunk/Source/WebCore/layout/MarginTypes.h 2019-01-28 15:43:00 UTC (rev 240582)
+++ trunk/Source/WebCore/layout/MarginTypes.h 2019-01-28 16:20:17 UTC (rev 240583)
@@ -84,8 +84,11 @@
struct PositiveAndNegativeVerticalMargin {
struct Values {
+ bool isNonZero() const { return positive.valueOr(0) || negative.valueOr(0); }
+
Optional<LayoutUnit> positive;
Optional<LayoutUnit> negative;
+ bool isQuirk { false };
};
Values before;
Values after;
Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h (240582 => 240583)
--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h 2019-01-28 15:43:00 UTC (rev 240582)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContext.h 2019-01-28 16:20:17 UTC (rev 240583)
@@ -123,6 +123,7 @@
static bool needsStretching(const LayoutState&, const Box&);
static HeightAndMargin stretchedInFlowHeight(const LayoutState&, const Box&, HeightAndMargin);
+ static bool shouldIgnoreCollapsedQuirkMargin(const LayoutState&, const Box&);
static bool shouldIgnoreMarginBefore(const LayoutState&, const Box&);
static bool shouldIgnoreMarginAfter(const LayoutState&, const Box&);
};
Modified: trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp (240582 => 240583)
--- trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp 2019-01-28 15:43:00 UTC (rev 240582)
+++ trunk/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp 2019-01-28 16:20:17 UTC (rev 240583)
@@ -48,11 +48,6 @@
return layoutBox.isBodyBox() || layoutBox.isDocumentBox() || layoutBox.isTableCell();
}
-static bool hasMarginBeforeQuirkValue(const Box& layoutBox)
-{
- return layoutBox.style().hasMarginBeforeQuirk();
-}
-
bool BlockFormattingContext::Quirks::needsStretching(const LayoutState& layoutState, const Box& layoutBox)
{
// In quirks mode, body stretches to html and html to the initial containing block (height: auto only).
@@ -104,29 +99,12 @@
return heightAndMargin;
}
-bool BlockFormattingContext::Quirks::shouldIgnoreMarginBefore(const LayoutState& layoutState, const Box& layoutBox)
+bool BlockFormattingContext::Quirks::shouldIgnoreCollapsedQuirkMargin(const LayoutState& layoutState, const Box& layoutBox)
{
- if (!layoutBox.parent())
- return false;
-
- return layoutState.inQuirksMode() && isQuirkContainer(*layoutBox.parent()) && hasMarginBeforeQuirkValue(layoutBox);
+ return layoutState.inQuirksMode() && isQuirkContainer(layoutBox);
}
-bool BlockFormattingContext::Quirks::shouldIgnoreMarginAfter(const LayoutState& layoutState, const Box& layoutBox)
-{
- // Ignore maring after when the ignored margin before collapses through.
- if (!shouldIgnoreMarginBefore(layoutState, layoutBox))
- return false;
-
- ASSERT(layoutBox.parent());
- auto& parent = *layoutBox.parent();
- if (parent.firstInFlowOrFloatingChild() != &layoutBox || parent.firstInFlowOrFloatingChild() != parent.lastInFlowOrFloatingChild())
- return false;
-
- return MarginCollapse::marginsCollapseThrough(layoutState, layoutBox);
}
-
}
-}
#endif
Modified: trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp (240582 => 240583)
--- trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp 2019-01-28 15:43:00 UTC (rev 240582)
+++ trunk/Source/WebCore/layout/blockformatting/BlockMarginCollapse.cpp 2019-01-28 16:20:17 UTC (rev 240583)
@@ -155,10 +155,6 @@
return false;
auto& previousInFlowSibling = *layoutBox.previousInFlowSibling();
-
- if (Quirks::shouldIgnoreMarginAfter(layoutState, previousInFlowSibling))
- return false;
-
// Margins between a floated box and any other box do not collapse.
if (layoutBox.isFloatingPositioned() || previousInFlowSibling.isFloatingPositioned())
return false;
@@ -203,9 +199,6 @@
return false;
auto& firstInFlowChild = *downcast<Container>(layoutBox).firstInFlowChild();
- if (Quirks::shouldIgnoreMarginBefore(layoutState, firstInFlowChild))
- return false;
-
if (!firstInFlowChild.isBlockLevelBox())
return false;
@@ -326,9 +319,6 @@
return false;
auto& lastInFlowChild = *downcast<Container>(layoutBox).lastInFlowChild();
- if (Quirks::shouldIgnoreMarginAfter(layoutState, lastInFlowChild))
- return false;
-
if (!lastInFlowChild.isBlockLevelBox())
return false;
@@ -452,21 +442,16 @@
else
computedValues.negative = a.negative ? a.negative : b.negative;
+ if (a.isNonZero() && b.isNonZero())
+ computedValues.isQuirk = a.isQuirk && b.isQuirk;
+ else if (a.isNonZero())
+ computedValues.isQuirk = a.isQuirk;
+ else
+ computedValues.isQuirk = b.isQuirk;
+
return computedValues;
}
-static PositiveAndNegativeVerticalMargin::Values computedPositiveAndNegativeMargin(PositiveAndNegativeVerticalMargin::Values a, Optional<LayoutUnit> marginValue)
-{
- if (!marginValue || !marginValue.value())
- return a;
- if (*marginValue > 0)
- return computedPositiveAndNegativeMargin(a, { marginValue, { } });
- if (*marginValue < 0)
- return computedPositiveAndNegativeMargin(a, { { }, marginValue });
- ASSERT_NOT_REACHED();
- return { };
-}
-
static Optional<LayoutUnit> marginValue(PositiveAndNegativeVerticalMargin::Values marginValues)
{
// When two or more margins collapse, the resulting margin width is the maximum of the collapsing margins' widths.
@@ -560,7 +545,16 @@
// 2. Gather positive and negative margin values from previous inflow sibling if margins are adjoining.
// 3. Compute min/max positive and negative collapsed margin values using non-collpased computed margin before.
auto collapsedMarginBefore = computedPositiveAndNegativeMargin(firstChildCollapsedMarginBefore(), previouSiblingCollapsedMarginAfter());
- return computedPositiveAndNegativeMargin(collapsedMarginBefore, nonCollapsedValues.before);
+ if (collapsedMarginBefore.isQuirk && Quirks::shouldIgnoreCollapsedQuirkMargin(layoutState, layoutBox))
+ collapsedMarginBefore = { };
+
+ PositiveAndNegativeVerticalMargin::Values nonCollapsedBefore;
+ if (nonCollapsedValues.before > 0)
+ nonCollapsedBefore = { nonCollapsedValues.before, { }, layoutBox.style().hasMarginBeforeQuirk() };
+ else if (nonCollapsedValues.before < 0)
+ nonCollapsedBefore = { { }, nonCollapsedValues.before, layoutBox.style().hasMarginBeforeQuirk() };
+
+ return computedPositiveAndNegativeMargin(collapsedMarginBefore, nonCollapsedBefore);
}
PositiveAndNegativeVerticalMargin::Values BlockFormattingContext::MarginCollapse::positiveNegativeMarginAfter(const LayoutState& layoutState, const Box& layoutBox, const UsedVerticalMargin::NonCollapsedValues& nonCollapsedValues)
@@ -573,7 +567,13 @@
// We don't know yet the margin before value of the next sibling. Let's just pretend it does not have one and
// update it later when we compute the next sibling's margin before. See updateCollapsedMarginAfter.
- return computedPositiveAndNegativeMargin(lastChildCollapsedMarginAfter(), nonCollapsedValues.after);
+ PositiveAndNegativeVerticalMargin::Values nonCollapsedAfter;
+ if (nonCollapsedValues.after > 0)
+ nonCollapsedAfter = { nonCollapsedValues.after, { }, layoutBox.style().hasMarginAfterQuirk() };
+ else if (nonCollapsedValues.after < 0)
+ nonCollapsedAfter = { { }, nonCollapsedValues.after, layoutBox.style().hasMarginAfterQuirk() };
+
+ return computedPositiveAndNegativeMargin(lastChildCollapsedMarginAfter(), nonCollapsedAfter);
}
EstimatedMarginBefore BlockFormattingContext::MarginCollapse::estimatedMarginBefore(const LayoutState& layoutState, const Box& layoutBox)
Modified: trunk/Tools/ChangeLog (240582 => 240583)
--- trunk/Tools/ChangeLog 2019-01-28 15:43:00 UTC (rev 240582)
+++ trunk/Tools/ChangeLog 2019-01-28 16:20:17 UTC (rev 240583)
@@ -1,3 +1,12 @@
+2019-01-28 Zalan Bujtas <[email protected]>
+
+ [LFC][MarginCollapsing][Quirks] Quirk margin values get propagated through margin collapsing
+ https://bugs.webkit.org/show_bug.cgi?id=193896
+
+ Reviewed by Antti Koivisto.
+
+ * LayoutReloaded/misc/LFC-passing-tests.txt:
+
2018-12-15 Darin Adler <[email protected]>
Replace many uses of String::format with more type-safe alternatives
Modified: trunk/Tools/LayoutReloaded/misc/LFC-passing-tests.txt (240582 => 240583)
--- trunk/Tools/LayoutReloaded/misc/LFC-passing-tests.txt 2019-01-28 15:43:00 UTC (rev 240582)
+++ trunk/Tools/LayoutReloaded/misc/LFC-passing-tests.txt 2019-01-28 16:20:17 UTC (rev 240583)
@@ -101,12 +101,15 @@
fast/block/basic/quirk-mode-percent-height.html
fast/block/basic/quirk-percent-height-grandchild.html
fast/block/float/001.html
+fast/block/float/003.html
fast/block/float/007.html
fast/block/float/008.html
fast/block/float/009.html
fast/block/float/013.html
fast/block/float/019.html
+fast/block/float/023.html
fast/block/float/030.html
+fast/block/float/033.html
fast/block/float/negative-margin-clear.html
fast/block/float/overhanging-after-height-decrease-offsets.html
fast/block/float/overhanging-after-height-decrease.html
@@ -125,8 +128,16 @@
fast/block/float/floats-with-negative-horizontal-margin.html
fast/block/float/float-forced-below-other-floats.html
fast/block/float/float-first-child-and-clear-sibling.html
+fast/block/float/intruding-float-sibling-with-margin.html
+fast/block/float/positioned-float-crash.html
+fast/block/float/previous-sibling-abspos-001.html
+fast/block/float/previous-sibling-abspos-002.html
+fast/block/float/previous-sibling-float-001.html
+fast/block/float/previous-sibling-float-002.html
fast/block/margin-collapse/002.html
fast/block/margin-collapse/003.html
+# webkit bug negative margin, document renderer size is invalid.
+#fast/block/margin-collapse/004.html
fast/block/margin-collapse/026.html
fast/block/margin-collapse/027.html
fast/block/margin-collapse/028.html
@@ -134,7 +145,9 @@
fast/block/margin-collapse/035.html
fast/block/margin-collapse/039.html
fast/block/margin-collapse/040.html
+fast/block/margin-collapse/043.html
fast/block/margin-collapse/044.html
+fast/block/margin-collapse/063.html
fast/block/margin-collapse/collapsed-through-child-simple.html
fast/block/positioning/003.html
fast/block/positioning/004.html
@@ -174,11 +187,16 @@
fast/block/positioning/044.html
fast/block/positioning/045.html
fast/block/positioning/046.html
+fast/block/positioning/048.html
fast/block/positioning/049.html
+fast/block/positioning/050.html
fast/block/positioning/052.html
fast/block/positioning/054.html
fast/block/positioning/060.html
+fast/block/positioning/abs-inside-inline-rel.html
+fast/block/positioning/absolute-length-of-neg-666666.html
fast/block/positioning/absolute-positioning-no-scrollbar.html
+fast/block/positioning/border-change-relayout-test.html
fast/block/positioning/change-containing-block-for-absolute-positioned.html
fast/block/positioning/change-containing-block-for-fixed-positioned.html
fast/block/positioning/complex-positioned-movement.html
@@ -192,6 +210,7 @@
fast/block/positioning/abs-inside-inline-rel.html
fast/block/positioning/pref-width-change.html
fast/block/positioning/relative-overflow-replaced-float.html
+fast/block/positioning/relative-overflow-replaced-float.html
fast/block/positioning/static-inline-position-dynamic.html
fast/block/inside-inlines/basic-float-intrusion.html
fast/block/inside-inlines/crash-on-first-line-change.html
