Attachment 348482 Details for Bug 189141 – Patch

[patch] Patch

bug-189141-20180829211534.patch (text/plain), 17.90 KB, created by zalan on 2018-08-29 21:15:35 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-08-29 21:15:35 PDT

Size: 17.90 KB

patch

obsolete

>Subversion Revision: 235460
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index acd1656c3fbd2879f25b5fac5274f96fdb04a3ca..5d3ea44e27144b92a6e59d87537ce180bc91a96a 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,41 @@
>+2018-08-29  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][Margins] Add non-computed horizontal margins to DisplayBox
>+        https://bugs.webkit.org/show_bug.cgi?id=189141
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Inflow block boxes' horizontal margins extend all the way to the left/right edge of their containing block.
>+        See https://www.w3.org/TR/CSS22/visudet.html#blockwidth for example
>+        "...
>+        10.3.3 Block-level, non-replaced elements in normal flow
>+        'margin-left' + 'border-left-width' + 'padding-left' + 'width' + 'padding-right' + 'border-right-width' + 'margin-right' = width of containing block
>+        ..."
>+
>+        In certain cases (float avoiding) we need to know the original (non-extended) horiztonal margin values.
>+
>+        * layout/FormattingContext.cpp:
>+        (WebCore::Layout::FormattingContext::computeFloatingWidthAndMargin const):
>+        (WebCore::Layout::FormattingContext::computeOutOfFlowHorizontalGeometry const):
>+        * layout/FormattingContextGeometry.cpp:
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGeometry):
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowReplacedHorizontalGeometry):
>+        (WebCore::Layout::FormattingContext::Geometry::floatingNonReplacedWidthAndMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::inlineReplacedWidthAndMargin):
>+        * layout/LayoutContext.cpp:
>+        (WebCore::Layout::LayoutContext::initializeRoot):
>+        * layout/LayoutUnits.h:
>+        * layout/blockformatting/BlockFormattingContext.cpp:
>+        (WebCore::Layout::BlockFormattingContext::computeInFlowWidthAndMargin const):
>+        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin):
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowReplacedWidthAndMargin):
>+        * layout/displaytree/DisplayBox.h:
>+        (WebCore::Display::Box::setHasValidHorizontalNonComputedMargin):
>+        (WebCore::Display::Box::setHorizontalNonComputedMargin):
>+        (WebCore::Display::Box::nonComputedMarginLeft const):
>+        (WebCore::Display::Box::nonComputedMarginRight const):
>+
> 2018-08-28  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][Floating] Introduce FloatBox, a FloatAvoider subclass.
>diff --git a/Source/WebCore/layout/FormattingContext.cpp b/Source/WebCore/layout/FormattingContext.cpp
>index de4800940226a02e102f725a4dfab49ef2e6699e..6f2c5c771294f3733904986f84fd76f61e8f733c 100644
>--- a/Source/WebCore/layout/FormattingContext.cpp
>+++ b/Source/WebCore/layout/FormattingContext.cpp
>@@ -67,6 +67,7 @@ void FormattingContext::computeFloatingWidthAndMargin(LayoutContext& layoutConte
>     displayBox.setContentBoxWidth(widthAndMargin.width);
>     displayBox.moveHorizontally(widthAndMargin.margin.left);
>     displayBox.setHorizontalMargin(widthAndMargin.margin);
>+    displayBox.setHorizontalNonComputedMargin(widthAndMargin.nonComputedMargin);
> }
> 
> void FormattingContext::computeOutOfFlowHorizontalGeometry(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
>@@ -75,6 +76,7 @@ void FormattingContext::computeOutOfFlowHorizontalGeometry(LayoutContext& layout
>     displayBox.setLeft(horizontalGeometry.left + horizontalGeometry.widthAndMargin.margin.left);
>     displayBox.setContentBoxWidth(horizontalGeometry.widthAndMargin.width);
>     displayBox.setHorizontalMargin(horizontalGeometry.widthAndMargin.margin);
>+    displayBox.setHorizontalNonComputedMargin(horizontalGeometry.widthAndMargin.nonComputedMargin);
> }
> 
> void FormattingContext::computeOutOfFlowVerticalGeometry(LayoutContext& layoutContext, const Box& layoutBox, Display::Box& displayBox) const
>diff --git a/Source/WebCore/layout/FormattingContextGeometry.cpp b/Source/WebCore/layout/FormattingContextGeometry.cpp
>index cc6d80a290bd728707649b444b4558c6722d7a6f..076e7466353aaca883e40ce2bf584904d404ea5e 100644
>--- a/Source/WebCore/layout/FormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/FormattingContextGeometry.cpp
>@@ -315,6 +315,8 @@ HorizontalGeometry FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGe
>     auto width = computedValueIfNotAuto(style.logicalWidth(), containingBlockWidth);
>     auto marginLeft = computedValueIfNotAuto(style.marginLeft(), containingBlockWidth);
>     auto marginRight = computedValueIfNotAuto(style.marginRight(), containingBlockWidth);
>+    auto nonComputedMarginLeft = marginLeft.value_or(0);
>+    auto nonComputedMarginRight = marginRight.value_or(0);
>     auto paddingLeft = displayBox.paddingLeft().value_or(0);
>     auto paddingRight = displayBox.paddingRight().value_or(0);
>     auto borderLeft = displayBox.borderLeft();
>@@ -415,7 +417,7 @@ HorizontalGeometry FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGe
>     ASSERT(marginRight);
> 
>     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Position][Width][Margin] -> out-of-flow non-replaced -> left(" << *left << "px) right("  << *right << "px) width(" << *width << "px) margin(" << *marginLeft << "px, "  << *marginRight << "px) layoutBox(" << &layoutBox << ")");
>-    return { *left, *right, { *width, { *marginLeft, *marginRight } } };
>+    return { *left, *right, { *width, { *marginLeft, *marginRight }, {nonComputedMarginLeft, nonComputedMarginRight } } };
> }
> 
> VerticalGeometry FormattingContext::Geometry::outOfFlowReplacedVerticalGeometry(LayoutContext& layoutContext, const Box& layoutBox)
>@@ -515,6 +517,8 @@ HorizontalGeometry FormattingContext::Geometry::outOfFlowReplacedHorizontalGeome
>     auto right = computedValueIfNotAuto(style.logicalRight(), containingBlockWidth);
>     auto marginLeft = computedValueIfNotAuto(style.marginLeft(), containingBlockWidth);
>     auto marginRight = computedValueIfNotAuto(style.marginRight(), containingBlockWidth);
>+    auto nonComputedMarginLeft = marginLeft.value_or(0);
>+    auto nonComputedMarginRight = marginRight.value_or(0);
>     auto width = inlineReplacedWidthAndMargin(layoutContext, layoutBox).width;
>     auto paddingLeft = displayBox.paddingLeft().value_or(0);
>     auto paddingRight = displayBox.paddingRight().value_or(0);
>@@ -580,7 +584,7 @@ HorizontalGeometry FormattingContext::Geometry::outOfFlowReplacedHorizontalGeome
>     ASSERT(marginRight);
> 
>     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Position][Width][Margin] -> out-of-flow replaced -> left(" << *left << "px) right("  << *right << "px) width(" << width << "px) margin(" << *marginLeft << "px, "  << *marginRight << "px) layoutBox(" << &layoutBox << ")");
>-    return { *left, *right, { width, { *marginLeft, *marginRight } } };
>+    return { *left, *right, { width, { *marginLeft, *marginRight }, { nonComputedMarginLeft, nonComputedMarginRight } } };
> }
> 
> HeightAndMargin FormattingContext::Geometry::complicatedCases(LayoutContext& layoutContext, const Box& layoutBox)
>@@ -646,7 +650,7 @@ WidthAndMargin FormattingContext::Geometry::floatingNonReplacedWidthAndMargin(La
>         width = shrinkToFitWidth(layoutContext, formattingContext, layoutBox);
> 
>     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Width][Margin] -> floating non-replaced -> width(" << *width << "px) margin(" << margin.left << "px, " << margin.right << "px) -> layoutBox(" << &layoutBox << ")");
>-    return WidthAndMargin { *width, margin };
>+    return WidthAndMargin { *width, margin, margin };
> }
> 
> HeightAndMargin FormattingContext::Geometry::floatingReplacedHeightAndMargin(LayoutContext& layoutContext, const Box& layoutBox)
>@@ -805,6 +809,8 @@ WidthAndMargin FormattingContext::Geometry::inlineReplacedWidthAndMargin(LayoutC
> 
>     auto marginLeft = computeMarginLeft();
>     auto marginRight = computeMarginRight();
>+    auto nonComputedMarginLeft = computedValueIfNotAuto(style.marginLeft(), containingBlockWidth).value_or(0);
>+    auto nonComputedMarginRight = computedValueIfNotAuto(style.marginRight(), containingBlockWidth).value_or(0);
>     auto width = computedValueIfNotAuto(style.logicalWidth(), containingBlockWidth);
> 
>     auto heightIsAuto = style.logicalHeight().isAuto();
>@@ -834,7 +840,7 @@ WidthAndMargin FormattingContext::Geometry::inlineReplacedWidthAndMargin(LayoutC
>     ASSERT(width);
> 
>     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Width][Margin] -> inflow replaced -> width(" << *width << "px) margin(" << marginLeft << "px, " << marginRight << "px) -> layoutBox(" << &layoutBox << ")");
>-    return { *width, { marginLeft, marginRight } };
>+    return { *width, { marginLeft, marginRight }, { nonComputedMarginLeft, nonComputedMarginRight } };
> }
> 
> Edges FormattingContext::Geometry::computedBorder(LayoutContext&, const Box& layoutBox)
>diff --git a/Source/WebCore/layout/LayoutContext.cpp b/Source/WebCore/layout/LayoutContext.cpp
>index 3cb6f9d5fca6e661945504670528ce67ab4273f8..552a7794a3f87522b1cbfe39cecd4b3134922f0b 100644
>--- a/Source/WebCore/layout/LayoutContext.cpp
>+++ b/Source/WebCore/layout/LayoutContext.cpp
>@@ -58,6 +58,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.setHorizontalNonComputedMargin({ });
>     displayBox.setVerticalMargin({ });
>     displayBox.setVerticalNonCollapsedMargin({ });
>     displayBox.setBorder({ });
>diff --git a/Source/WebCore/layout/LayoutUnits.h b/Source/WebCore/layout/LayoutUnits.h
>index 1b61edb3722313fec2ee4c0a1842d4792df7fb1d..4a925c9af9d591b1caffb9cd381443a34c63f423 100644
>--- a/Source/WebCore/layout/LayoutUnits.h
>+++ b/Source/WebCore/layout/LayoutUnits.h
>@@ -89,6 +89,7 @@ struct Edges {
> struct WidthAndMargin {
>     LayoutUnit width;
>     HorizontalEdges margin;
>+    HorizontalEdges nonComputedMargin;
> };
> 
> struct HeightAndMargin {
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>index 6edbd9ee002bc0e4da8a90b081edc7d393f596f7..27a1b4c2c4f3247dc9ab49754b20fd9d18098988 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>@@ -261,6 +261,7 @@ void BlockFormattingContext::computeInFlowWidthAndMargin(LayoutContext& layoutCo
>     displayBox.setContentBoxWidth(widthAndMargin.width);
>     displayBox.moveHorizontally(widthAndMargin.margin.left);
>     displayBox.setHorizontalMargin(widthAndMargin.margin);
>+    displayBox.setHorizontalNonComputedMargin(widthAndMargin.nonComputedMargin);
> }
> 
> FormattingContext::InstrinsicWidthConstraints BlockFormattingContext::instrinsicWidthConstraints(LayoutContext& layoutContext, const Box& layoutBox) const
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>index b8142d8d8cd3ae6961e7c677ca4f363057b941d5..4b96789e34c75cb4652856f3bccdbeb74b1cfcc9 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>@@ -189,6 +189,8 @@ WidthAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin
>         auto width = FormattingContext::Geometry::computedValueIfNotAuto(precomputedWidth ? Length { precomputedWidth.value(), Fixed } : style.logicalWidth(), containingBlockWidth);
>         auto marginLeft = FormattingContext::Geometry::computedValueIfNotAuto(style.marginLeft(), containingBlockWidth);
>         auto marginRight = FormattingContext::Geometry::computedValueIfNotAuto(style.marginRight(), containingBlockWidth);
>+        auto nonComputedMarginLeft = marginLeft.value_or(0);
>+        auto nonComputedMarginRight = marginRight.value_or(0);
>         auto borderLeft = displayBox.borderLeft();
>         auto borderRight = displayBox.borderRight();
>         auto paddingLeft = displayBox.paddingLeft().value_or(0);
>@@ -236,7 +238,7 @@ WidthAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin
>         ASSERT(marginLeft);
>         ASSERT(marginRight);
> 
>-        return WidthAndMargin { *width, { *marginLeft, *marginRight } };
>+        return WidthAndMargin { *width, { *marginLeft, *marginRight }, { nonComputedMarginLeft, nonComputedMarginRight } };
>     };
> 
>     auto widthAndMargin = compute();
>@@ -264,10 +266,10 @@ WidthAndMargin BlockFormattingContext::Geometry::inFlowReplacedWidthAndMargin(La
>     // #1
>     auto width = FormattingContext::Geometry::inlineReplacedWidthAndMargin(layoutContext, layoutBox).width;
>     // #2
>-    auto margin = inFlowNonReplacedWidthAndMargin(layoutContext, layoutBox, width).margin;
>+    auto nonReplacedWidthAndMargin = inFlowNonReplacedWidthAndMargin(layoutContext, layoutBox, width);
> 
>-    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Width][Margin] -> inflow replaced -> width(" << width << "px) margin(" << margin.left << "px, " << margin.left << "px) -> layoutBox(" << &layoutBox << ")");
>-    return { width, margin };
>+    LOG_WITH_STREAM(FormattingContextLayout, stream << "[Width][Margin] -> inflow replaced -> width(" << width << "px) margin(" << nonReplacedWidthAndMargin.margin.left << "px, " << nonReplacedWidthAndMargin.margin.right << "px) -> layoutBox(" << &layoutBox << ")");
>+    return { width, nonReplacedWidthAndMargin.margin, nonReplacedWidthAndMargin.nonComputedMargin };
> }
> 
> Position BlockFormattingContext::Geometry::staticPosition(LayoutContext& layoutContext, const Box& layoutBox)
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.cpp b/Source/WebCore/layout/displaytree/DisplayBox.cpp
>index 718b16eaf5023dc4b10e919aa3dc470f270a998a..1534793b78bba3ed23b60a5d82bac60acbd1bd03 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.cpp
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.cpp
>@@ -59,6 +59,7 @@ Box::Box(const Box& other)
>     , m_contentHeight(other.m_contentHeight)
>     , m_margin(other.m_margin)
>     , m_verticalNonCollapsedMargin(other.m_verticalNonCollapsedMargin)
>+    , m_horizontalNonComputedMargin(other.m_horizontalNonComputedMargin)
>     , m_estimatedMarginTop(other.m_estimatedMarginTop)
>     , m_border(other.m_border)
>     , m_padding(other.m_padding)
>@@ -68,6 +69,7 @@ Box::Box(const Box& other)
>     , m_hasValidHorizontalMargin(other.m_hasValidHorizontalMargin)
>     , m_hasValidVerticalMargin(other.m_hasValidVerticalMargin)
>     , m_hasValidVerticalNonCollapsedMargin(other.m_hasValidVerticalNonCollapsedMargin)
>+    , m_hasValidHorizontalNonComputedMargin(other.m_hasValidHorizontalNonComputedMargin)
>     , m_hasValidBorder(other.m_hasValidBorder)
>     , m_hasValidPadding(other.m_hasValidPadding)
>     , m_hasValidContentHeight(other.m_hasValidContentHeight)
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.h b/Source/WebCore/layout/displaytree/DisplayBox.h
>index e58f4869cb2d8e5a0f76defefa06ccd6ba2cd261..aef24e4797d00a3f7102b5813c5d402f9cbdd02f 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.h
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.h
>@@ -142,6 +142,9 @@ public:
> 
>     LayoutUnit nonCollapsedMarginTop() const;
>     LayoutUnit nonCollapsedMarginBottom() const;
>+    LayoutUnit nonComputedMarginLeft() const;
>+    LayoutUnit nonComputedMarginRight() const;
>+
>     std::optional<LayoutUnit> estimatedMarginTop() const { return m_estimatedMarginTop; }
> 
>     LayoutUnit borderTop() const;
>@@ -189,6 +192,7 @@ private:
>     void setHorizontalMargin(Layout::HorizontalEdges);
>     void setVerticalMargin(Layout::VerticalEdges);
>     void setVerticalNonCollapsedMargin(Layout::VerticalEdges);
>+    void setHorizontalNonComputedMargin(Layout::HorizontalEdges);
>     void setEstimatedMarginTop(LayoutUnit marginTop) { m_estimatedMarginTop = marginTop; }
> 
>     void setBorder(Layout::Edges);
>@@ -203,6 +207,7 @@ private:
>     void setHasValidLeft() { m_hasValidLeft = true; }
>     void setHasValidVerticalMargin() { m_hasValidVerticalMargin = true; }
>     void setHasValidVerticalNonCollapsedMargin() { m_hasValidVerticalNonCollapsedMargin = true; }
>+    void setHasValidHorizontalNonComputedMargin() { m_hasValidHorizontalNonComputedMargin = true; }
>     void setHasValidHorizontalMargin() { m_hasValidHorizontalMargin = true; }
> 
>     void setHasValidBorder() { m_hasValidBorder = true; }
>@@ -220,6 +225,7 @@ private:
> 
>     Layout::Edges m_margin;
>     Layout::VerticalEdges m_verticalNonCollapsedMargin;
>+    Layout::HorizontalEdges m_horizontalNonComputedMargin;
>     std::optional<LayoutUnit> m_estimatedMarginTop;
> 
>     Layout::Edges m_border;
>@@ -231,6 +237,7 @@ private:
>     bool m_hasValidHorizontalMargin { false };
>     bool m_hasValidVerticalMargin { false };
>     bool m_hasValidVerticalNonCollapsedMargin { false };
>+    bool m_hasValidHorizontalNonComputedMargin { false };
>     bool m_hasValidBorder { false };
>     bool m_hasValidPadding { false };
>     bool m_hasValidContentHeight { false };
>@@ -524,6 +531,14 @@ inline void Box::setVerticalNonCollapsedMargin(Layout::VerticalEdges margin)
>     m_verticalNonCollapsedMargin = margin;
> }
> 
>+inline void Box::setHorizontalNonComputedMargin(Layout::HorizontalEdges margin)
>+{
>+#if !ASSERT_DISABLED
>+    setHasValidHorizontalNonComputedMargin();
>+#endif
>+    m_horizontalNonComputedMargin = margin;
>+}
>+
> inline void Box::setBorder(Layout::Edges border)
> {
> #if !ASSERT_DISABLED
>@@ -576,6 +591,18 @@ inline LayoutUnit Box::nonCollapsedMarginBottom() const
>     return m_verticalNonCollapsedMargin.bottom;
> }
> 
>+inline LayoutUnit Box::nonComputedMarginLeft() const
>+{
>+    ASSERT(m_hasValidHorizontalNonComputedMargin);
>+    return m_horizontalNonComputedMargin.left;
>+}
>+
>+inline LayoutUnit Box::nonComputedMarginRight() const
>+{
>+    ASSERT(m_hasValidHorizontalNonComputedMargin);
>+    return m_horizontalNonComputedMargin.right;
>+}
>+
> inline std::optional<LayoutUnit> Box::paddingTop() const
> {
>     ASSERT(m_hasValidPadding);

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 189141: 348473 | 348482