Attachment 343894 Details for Bug 187169 – Patch

[patch] Patch

bug-187169-20180628221915.patch (text/plain), 11.91 KB, created by zalan on 2018-06-28 22:19:16 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-06-28 22:19:16 PDT

Size: 11.91 KB

patch

obsolete

>Subversion Revision: 233348
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index dcca264af52139f88c49f3d291de7dd6d3619de9..4a89c7c3381fcf3a069c0ec53d23484909fbb31d 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2018-06-28  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] The static position for an out-of-flow box should include the previous sibling's collapsed margin
>+        https://bugs.webkit.org/show_bug.cgi?id=187169
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When computing the static position of an absolutely positioned box, we need to look at the previous sibling's bottom margin.
>+        If the previous sibling happens to collapse its bottom margin with the parent's bottom margin, we still need to account for it
>+        and compute the static vertical position as if the bottom margin was not collapsed.
>+
>+        * layout/FormattingContext.cpp:
>+        (WebCore::Layout::FormattingContext::computeFloatingHeightAndMargin const):
>+        (WebCore::Layout::FormattingContext::computeOutOfFlowVerticalGeometry const):
>+        * layout/FormattingContextGeometry.cpp:
>+        (WebCore::Layout::staticVerticalPositionForOutOfFlowPositioned):
>+        * layout/LayoutContext.cpp:
>+        (WebCore::Layout::LayoutContext::initializeRoot):
>+        * layout/Verification.cpp:
>+        (WebCore::Layout::outputMismatchingBoxInformationIfNeeded):
>+        * layout/blockformatting/BlockFormattingContext.cpp:
>+        (WebCore::Layout::BlockFormattingContext::computeInFlowHeightAndMargin const):
>+        * layout/displaytree/DisplayBox.cpp:
>+        (WebCore::Display::Box::nonCollapsedMarginBox const):
>+        * layout/displaytree/DisplayBox.h:
>+        (WebCore::Display::Box::setHasValidVerticalNonCollapsedMargin):
>+        (WebCore::Display::Box::setVerticalMargin):
>+        (WebCore::Display::Box::setVerticalNonCollapsedMargin):
>+        (WebCore::Display::Box::nonCollapsedMarginTop const):
>+        (WebCore::Display::Box::nonCollapsedMarginBottom const):
>+
> 2018-06-28  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Out-of-flow positioned height does not necessarily equal to "bottom - top".
>diff --git a/Source/WebCore/layout/FormattingContext.cpp b/Source/WebCore/layout/FormattingContext.cpp
>index f284d1ecdb985036a1ad71d64d07e28b3ee7af6c..c420a84499f134424e120675101c428f69e70123 100644
>--- a/Source/WebCore/layout/FormattingContext.cpp
>+++ b/Source/WebCore/layout/FormattingContext.cpp
>@@ -58,6 +58,7 @@ void FormattingContext::computeFloatingHeightAndMargin(LayoutContext& layoutCont
>     displayBox.moveVertically(heightAndMargin.margin.top);
>     ASSERT(!heightAndMargin.collapsedMargin);
>     displayBox.setVerticalMargin(heightAndMargin.margin);
>+    displayBox.setVerticalNonCollapsedMargin(heightAndMargin.margin);
> }
> 
> void FormattingContext::computeFloatingWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
>@@ -83,6 +84,7 @@ void FormattingContext::computeOutOfFlowVerticalGeometry(LayoutContext& layoutCo
>     displayBox.setContentBoxHeight(verticalGeometry.heightAndMargin.height);
>     ASSERT(!verticalGeometry.heightAndMargin.collapsedMargin);
>     displayBox.setVerticalMargin(verticalGeometry.heightAndMargin.margin);
>+    displayBox.setVerticalNonCollapsedMargin(verticalGeometry.heightAndMargin.margin);
> }
> 
> void FormattingContext::computeBorderAndPadding(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
>diff --git a/Source/WebCore/layout/FormattingContextGeometry.cpp b/Source/WebCore/layout/FormattingContextGeometry.cpp
>index c62b233950976530d0dc79066e17dc2bcace1607..5df266437adba8614337e4fa795ee29c746741b3 100644
>--- a/Source/WebCore/layout/FormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/FormattingContextGeometry.cpp
>@@ -80,6 +80,11 @@ static LayoutUnit staticVerticalPositionForOutOfFlowPositioned(const LayoutConte
>     ASSERT(layoutBox.isOutOfFlowPositioned());
> 
>     LayoutUnit top;
>+    // Add sibling offset
>+    if (auto* previousInFlowSibling = layoutBox.previousInFlowSibling()) {
>+        auto& previousInFlowDisplayBox = *layoutContext.displayBoxForLayoutBox(*previousInFlowSibling);
>+        top += previousInFlowDisplayBox.bottom() + previousInFlowDisplayBox.nonCollapsedMarginBottom();
>+    }
>     // Resolve top all the way up to the containing block.
>     auto* containingBlock = layoutBox.containingBlock();
>     for (auto* parent = layoutBox.parent(); parent; parent = parent->parent()) {
>@@ -88,11 +93,6 @@ static LayoutUnit staticVerticalPositionForOutOfFlowPositioned(const LayoutConte
>         if (parent == containingBlock)
>             break;
>     }
>-    // Add sibling offset
>-    if (auto* previousInFlowSibling = layoutBox.previousInFlowSibling()) {
>-        auto& previousInFlowDisplayBox = *layoutContext.displayBoxForLayoutBox(*previousInFlowSibling);
>-        top += previousInFlowDisplayBox.bottom() + previousInFlowDisplayBox.marginBottom();
>-    }
>     // FIXME: floatings need to be taken into account.
>     return top;
> }
>diff --git a/Source/WebCore/layout/LayoutContext.cpp b/Source/WebCore/layout/LayoutContext.cpp
>index 9999617da2d9d0f7fd68896223745edce55d3d03..751eb6241d61ecfec69d406359aeaa8f9199d693 100644
>--- a/Source/WebCore/layout/LayoutContext.cpp
>+++ b/Source/WebCore/layout/LayoutContext.cpp
>@@ -59,6 +59,7 @@ void LayoutContext::initializeRoot(const Container& root, const LayoutSize& cont
>     // FIXME: m_root could very well be a formatting context root with ancestors and resolvable border and padding (as opposed to the topmost root)
>     displayBox.setHorizontalMargin({ });
>     displayBox.setVerticalMargin({ });
>+    displayBox.setVerticalNonCollapsedMargin({ });
>     displayBox.setBorder({ });
>     displayBox.setPadding({ });
>     displayBox.setContentBoxHeight(containerSize.height());
>diff --git a/Source/WebCore/layout/Verification.cpp b/Source/WebCore/layout/Verification.cpp
>index 6697ea82c5747fbb32b5b1373a547668e7949d14..48164a072952d29b878b3303d5880b4e39175c4f 100644
>--- a/Source/WebCore/layout/Verification.cpp
>+++ b/Source/WebCore/layout/Verification.cpp
>@@ -67,19 +67,10 @@ static bool outputMismatchingBoxInformationIfNeeded(TextStream& stream, const La
>         return true;
>     }
> 
>-#ifndef NDEBUG
>     if (renderer.marginBoxRect() != displayBox->nonCollapsedMarginBox()) {
>         outputRect("marginBox", renderer.marginBoxRect(), displayBox->nonCollapsedMarginBox());
>         return true;
>     }
>-#else
>-    // For now in non-debug builds, verify the horizontal margin only
>-    if (renderer.marginBoxRect().left() != displayBox->marginBox().left()
>-        || renderer.marginBoxRect().right() != displayBox->marginBox().right() ) {
>-        outputRect("marginBox", renderer.marginBoxRect(), displayBox->marginBox());
>-        return true;
>-    }
>-#endif
> 
>     if (renderer.borderBoxRect() != displayBox->borderBox()) {
>         outputRect("borderBox", renderer.borderBoxRect(), displayBox->borderBox());
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>index c9c8275751d4fa3d86f5aedbf739806d088a342f..2a057c6f9c8c328ce2bb14f52e9e7012d8a78167 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>@@ -194,9 +194,7 @@ void BlockFormattingContext::computeInFlowHeightAndMargin(LayoutContext& layoutC
>     displayBox.setContentBoxHeight(heightAndMargin.height);
>     displayBox.moveVertically(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin).top);
>     displayBox.setVerticalMargin(heightAndMargin.collapsedMargin.value_or(heightAndMargin.margin));
>-#ifndef NDEBUG
>     displayBox.setVerticalNonCollapsedMargin(heightAndMargin.margin);
>-#endif
> }
> 
> void BlockFormattingContext::computeInFlowWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.cpp b/Source/WebCore/layout/displaytree/DisplayBox.cpp
>index dc1f31bb00d29d438c156621d9bd875a4a8fb14f..b020c1e72834f1ad1e3ce1ce24299adb5ae27add 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.cpp
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.cpp
>@@ -62,19 +62,17 @@ Box::Rect Box::marginBox() const
>     return marginBox;
> }
> 
>-#ifndef NDEBUG
> Box::Rect Box::nonCollapsedMarginBox() const
> {
>     auto borderBox = this->borderBox();
> 
>     Rect marginBox;
>-    marginBox.setTop(borderBox.top() - m_nonCollapsedVertivalMargin.top);
>+    marginBox.setTop(borderBox.top() - nonCollapsedMarginTop());
>     marginBox.setLeft(borderBox.left() - marginLeft());
>-    marginBox.setHeight(borderBox.height() + m_nonCollapsedVertivalMargin.top + m_nonCollapsedVertivalMargin.bottom);
>+    marginBox.setHeight(borderBox.height() + nonCollapsedMarginTop() + nonCollapsedMarginBottom());
>     marginBox.setWidth(borderBox.width() + marginLeft() + marginRight());
>     return marginBox;
> }
>-#endif
> 
> Box::Rect Box::borderBox() const
> {
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.h b/Source/WebCore/layout/displaytree/DisplayBox.h
>index 52bd9aa4778d7cb87403a4c42eea02a81f86ede7..758023cf187835670fe68de4d1945a4f9c461d62 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.h
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.h
>@@ -131,6 +131,9 @@ public:
>     LayoutUnit marginBottom() const;
>     LayoutUnit marginRight() const;
> 
>+    LayoutUnit nonCollapsedMarginTop() const;
>+    LayoutUnit nonCollapsedMarginBottom() const;
>+
>     LayoutUnit borderTop() const;
>     LayoutUnit borderLeft() const;
>     LayoutUnit borderBottom() const;
>@@ -147,9 +150,8 @@ public:
>     LayoutUnit contentBoxWidth() const;
> 
>     Rect marginBox() const;
>-#ifndef NDEBUG
>     Rect nonCollapsedMarginBox() const;
>-#endif
>+
>     Rect borderBox() const;
>     Rect paddingBox() const;
>     Rect contentBox() const;
>@@ -189,9 +191,8 @@ private:
> 
>     void setHorizontalMargin(HorizontalEdges);
>     void setVerticalMargin(VerticalEdges);
>-#ifndef NDEBUG
>-    void setVerticalNonCollapsedMargin(VerticalEdges margin) {  m_nonCollapsedVertivalMargin = margin; }
>-#endif
>+    void setVerticalNonCollapsedMargin(VerticalEdges);
>+
>     void setBorder(Edges);
>     void setPadding(Edges);
> 
>@@ -201,6 +202,7 @@ private:
>     void invalidatePadding() { m_hasValidPadding = false; }
> 
>     void setHasValidVerticalMargin() { m_hasValidVerticalMargin = true; }
>+    void setHasValidVerticalNonCollapsedMargin() { m_hasValidVerticalNonCollapsedMargin = true; }
>     void setHasValidHorizontalMargin() { m_hasValidHorizontalMargin = true; }
> 
>     void setHasValidBorder() { m_hasValidBorder = true; }
>@@ -217,15 +219,15 @@ private:
>     LayoutUnit m_contentHeight;
> 
>     Edges m_margin;
>-#ifndef NDEBUG
>-    VerticalEdges m_nonCollapsedVertivalMargin;
>-#endif
>+    VerticalEdges m_verticalNonCollapsedMargin;
>+
>     Edges m_border;
>     Edges m_padding;
> 
> #if !ASSERT_DISABLED
>     bool m_hasValidHorizontalMargin { false };
>     bool m_hasValidVerticalMargin { false };
>+    bool m_hasValidVerticalNonCollapsedMargin { false };
>     bool m_hasValidBorder { false };
>     bool m_hasValidPadding { false };
>     bool m_hasValidContentHeight { false };
>@@ -466,6 +468,14 @@ inline void Box::setVerticalMargin(VerticalEdges margin)
>     m_margin.vertical = margin;
> }
> 
>+inline void Box::setVerticalNonCollapsedMargin(VerticalEdges margin)
>+{
>+#if !ASSERT_DISABLED
>+    setHasValidVerticalNonCollapsedMargin();
>+#endif
>+    m_verticalNonCollapsedMargin = margin;
>+}
>+
> inline void Box::setBorder(Edges border)
> {
> #if !ASSERT_DISABLED
>@@ -506,6 +516,18 @@ inline LayoutUnit Box::marginRight() const
>     return m_margin.horizontal.right;
> }
> 
>+inline LayoutUnit Box::nonCollapsedMarginTop() const
>+{
>+    ASSERT(m_hasValidVerticalNonCollapsedMargin);
>+    return m_verticalNonCollapsedMargin.top;
>+}
>+
>+inline LayoutUnit Box::nonCollapsedMarginBottom() const
>+{
>+    ASSERT(m_hasValidVerticalNonCollapsedMargin);
>+    return m_verticalNonCollapsedMargin.bottom;
>+}
>+
> inline LayoutUnit Box::paddingTop() const
> {
>     ASSERT(m_hasValidPadding);

Actions: View | Formatted Diff | Diff

Attachments on bug 187169: 343894