Attachment 356445 Details for Bug 192345 – Patch

[patch] Patch

Patch.txt (text/plain), 10.77 KB, created by zalan on 2018-12-03 19:02:16 PST

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-12-03 19:02:16 PST

Size: 10.77 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 6d9eef69724..01aee25f18f 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,26 @@
>+2018-12-03  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][BFC][MarginCollapsing] HeightAndMargin::margin is always the non-collapsed margin value.
>+        https://bugs.webkit.org/show_bug.cgi?id=192345
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Rename HeightAndMargin::margin to HeightAndMargin::nonCollapsedMargin.
>+
>+        * layout/FormattingContext.cpp:
>+        (WebCore::Layout::FormattingContext::computeOutOfFlowVerticalGeometry const):
>+        * layout/LayoutUnits.h:
>+        (WebCore::Layout::HeightAndMargin::usedMarginValues const):
>+        * 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):
>+
> 2018-12-02  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][BFC][MarginCollapsing] Add MarginCollapse::establishesBlockFormattingContext
>diff --git a/Source/WebCore/layout/FormattingContext.cpp b/Source/WebCore/layout/FormattingContext.cpp
>index 0f8b02c8c40..ff41200ae7b 100644
>--- a/Source/WebCore/layout/FormattingContext.cpp
>+++ b/Source/WebCore/layout/FormattingContext.cpp
>@@ -115,11 +115,13 @@ void FormattingContext::computeOutOfFlowVerticalGeometry(const Box& layoutBox) c
>     }
> 
>     auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
>-    displayBox.setTop(verticalGeometry.top + verticalGeometry.heightAndMargin.margin.top);
>-    displayBox.setContentBoxHeight(verticalGeometry.heightAndMargin.height);
>+    // Margins of absolutely positioned boxes do not collapse
>     ASSERT(!verticalGeometry.heightAndMargin.collapsedMargin);
>-    displayBox.setVerticalMargin(verticalGeometry.heightAndMargin.margin);
>-    displayBox.setVerticalNonCollapsedMargin(verticalGeometry.heightAndMargin.margin);
>+    auto nonCollapsedVerticalMargins = verticalGeometry.heightAndMargin.usedMarginValues();
>+    displayBox.setTop(verticalGeometry.top + nonCollapsedVerticalMargins.top);
>+    displayBox.setContentBoxHeight(verticalGeometry.heightAndMargin.height);
>+    displayBox.setVerticalMargin(nonCollapsedVerticalMargins);
>+    displayBox.setVerticalNonCollapsedMargin(nonCollapsedVerticalMargins);
> }
> 
> void FormattingContext::computeBorderAndPadding(const Box& layoutBox) const
>diff --git a/Source/WebCore/layout/LayoutUnits.h b/Source/WebCore/layout/LayoutUnits.h
>index b1c3d521d45..3e1d8446f75 100644
>--- a/Source/WebCore/layout/LayoutUnits.h
>+++ b/Source/WebCore/layout/LayoutUnits.h
>@@ -107,10 +107,10 @@ struct WidthAndMargin {
> };
> 
> struct HeightAndMargin {
>-    VerticalEdges usedMarginValues() const { return collapsedMargin.value_or(margin); }
>+    VerticalEdges usedMarginValues() const { return collapsedMargin.value_or(nonCollapsedMargin); }
> 
>     LayoutUnit height;
>-    VerticalEdges margin;
>+    VerticalEdges nonCollapsedMargin;
>     std::optional<VerticalEdges> collapsedMargin;
> };
> 
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>index f80e31ff2dd..26a82b73354 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>@@ -358,7 +358,7 @@ void BlockFormattingContext::computeHeightAndMargin(const Box& layoutBox) const
>             auto maxHeightAndMargin = compute(maxHeight);
>             // Used height should remain the same.
>             ASSERT((layoutState.inQuirksMode() && (layoutBox.isBodyBox() || layoutBox.isDocumentBox())) || maxHeightAndMargin.height == *maxHeight);
>-            heightAndMargin = { *maxHeight, maxHeightAndMargin.margin, maxHeightAndMargin.collapsedMargin };
>+            heightAndMargin = { *maxHeight, maxHeightAndMargin.nonCollapsedMargin, maxHeightAndMargin.collapsedMargin };
>         }
>     }
> 
>@@ -367,18 +367,18 @@ void BlockFormattingContext::computeHeightAndMargin(const Box& layoutBox) const
>             auto minHeightAndMargin = compute(minHeight);
>             // Used height should remain the same.
>             ASSERT((layoutState.inQuirksMode() && (layoutBox.isBodyBox() || layoutBox.isDocumentBox())) || minHeightAndMargin.height == *minHeight);
>-            heightAndMargin = { *minHeight, minHeightAndMargin.margin, minHeightAndMargin.collapsedMargin };
>+            heightAndMargin = { *minHeight, minHeightAndMargin.nonCollapsedMargin, minHeightAndMargin.collapsedMargin };
>         }
>     }
> 
>     auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
>     displayBox.setContentBoxHeight(heightAndMargin.height);
>-    displayBox.setVerticalNonCollapsedMargin(heightAndMargin.margin);
>-    displayBox.setVerticalMargin(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin));
>+    displayBox.setVerticalNonCollapsedMargin(heightAndMargin.nonCollapsedMargin);
>+    displayBox.setVerticalMargin(heightAndMargin.usedMarginValues());
> 
>     // If this box has already been moved by the estimated vertical margin, no need to move it again.
>     if (layoutBox.isFloatingPositioned() || !displayBox.estimatedMarginTop())
>-        displayBox.moveVertically(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin).top);
>+        displayBox.moveVertically(heightAndMargin.usedMarginValues().top);
> }
> 
> FormattingContext::InstrinsicWidthConstraints BlockFormattingContext::instrinsicWidthConstraints() const
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>index 819f0303e09..3b09f30cf1b 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>@@ -104,7 +104,7 @@ HeightAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMarg
> 
>     auto heightAndMargin = compute();
> 
>-    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> height(" << heightAndMargin.height << "px) margin(" << heightAndMargin.margin.top << "px, " << heightAndMargin.margin.bottom << "px) -> layoutBox(" << &layoutBox << ")");
>+    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> height(" << heightAndMargin.height << "px) margin(" << heightAndMargin.usedMarginValues().top << "px, " << heightAndMargin.usedMarginValues().bottom << "px) -> layoutBox(" << &layoutBox << ")");
>     return heightAndMargin;
> }
> 
>@@ -263,7 +263,7 @@ HeightAndMargin BlockFormattingContext::Geometry::inFlowHeightAndMargin(const La
> 
>     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.margin.top << "px, " << heightAndMargin.margin.bottom << "px) -> layoutBox(" << &layoutBox << ")");
>+    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> inflow non-replaced -> streched to viewport -> height(" << heightAndMargin.height << "px) margin(" << heightAndMargin.usedMarginValues().top << "px, " << heightAndMargin.usedMarginValues().bottom << "px) -> layoutBox(" << &layoutBox << ")");
>     return heightAndMargin;
> }
> 
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp
>index 2ab92e89204..9372cbc02d1 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextQuirks.cpp
>@@ -79,9 +79,12 @@ HeightAndMargin BlockFormattingContext::Quirks::stretchedHeight(const LayoutStat
>     strechedHeight -= documentBoxVerticalBorders + documentBoxVerticalPaddings;
> 
>     LayoutUnit totalVerticalMargins;
>-    if (layoutBox.isDocumentBox())
>-        totalVerticalMargins = heightAndMargin.margin.top + heightAndMargin.margin.bottom;
>-    else if (layoutBox.isBodyBox()) {
>+    if (layoutBox.isDocumentBox()) {
>+        auto verticalMargins = heightAndMargin.usedMarginValues();
>+        // Document box's margins do not collapse.
>+        ASSERT(!heightAndMargin.collapsedMargin);
>+        totalVerticalMargins = verticalMargins.top + verticalMargins.bottom;
>+    } 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.
>         // This looks extremely odd when html has non-auto height.
>@@ -90,7 +93,7 @@ HeightAndMargin BlockFormattingContext::Quirks::stretchedHeight(const LayoutStat
> 
>         // 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.
>-        auto bodyBoxVerticalMargins = heightAndMargin.height ? heightAndMargin.usedMarginValues() : heightAndMargin.margin;
>+        auto bodyBoxVerticalMargins = heightAndMargin.height ? heightAndMargin.usedMarginValues() : heightAndMargin.nonCollapsedMargin;
>         totalVerticalMargins = bodyBoxVerticalMargins.top + bodyBoxVerticalMargins.bottom;
>     }
> 
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp
>index 32012c3ca2a..b530ad6f4c1 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContext.cpp
>@@ -362,8 +362,8 @@ void InlineFormattingContext::computeHeightAndMargin(const Box& layoutBox) const
> 
>     auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
>     displayBox.setContentBoxHeight(heightAndMargin.height);
>-    displayBox.setVerticalNonCollapsedMargin(heightAndMargin.margin);
>-    displayBox.setVerticalMargin(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin));
>+    displayBox.setVerticalNonCollapsedMargin(heightAndMargin.nonCollapsedMargin);
>+    displayBox.setVerticalMargin(heightAndMargin.usedMarginValues());
> }
> 
> void InlineFormattingContext::layoutFormattingContextRoot(const Box& root) const

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 192345: 356445