Attachment 342726 Details for Bug 186616 – Patch

[patch] Patch

bug-186616-20180613221332.patch (text/plain), 13.17 KB, created by zalan on 2018-06-13 22:13:33 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-06-13 22:13:33 PDT

Size: 13.17 KB

patch

obsolete

>Subversion Revision: 232752
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index f70db967917edc479e9e12a31269d4ba6e48fd01..74bfd78286b2af32cf20341af250b2c2be1e6e37 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,42 @@
>+2018-06-13  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] Miscellaneous fixes to be able to layout <div> with fixed width/height
>+        https://bugs.webkit.org/show_bug.cgi?id=186616
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        1. Move box horizontally/vertically when relevant margin is computed.
>+        2. Fix isStretchedToViewport() logic and make sure that the width/height is adjusted with the margin when the box is stretchy.
>+        3. Fix isPaddingApplicable() and add "GenericElement" for elements that don't need special handling.
>+
>+        With this patch LFC produces the correct geometry for the following content:
>+        <html><body><div style="width: 100px; height: 100px;"></div></body></html> 
>+
>+        * layout/FormattingContext.cpp:
>+        (WebCore::Layout::FormattingContext::computeFloatingHeightAndMargin const):
>+        (WebCore::Layout::FormattingContext::computeFloatingWidthAndMargin const):
>+        * layout/blockformatting/BlockFormattingContext.cpp:
>+        (WebCore::Layout::BlockFormattingContext::layout const):
>+        (WebCore::Layout::BlockFormattingContext::computeInFlowHeightAndMargin const):
>+        (WebCore::Layout::BlockFormattingContext::computeInFlowWidthAndMargin const):
>+        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
>+        (WebCore::Layout::isStretchedToViewport):
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedHeightAndMargin):
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin):
>+        (WebCore::Layout::BlockFormattingContext::Geometry::staticPosition):
>+        * layout/displaytree/DisplayBox.h:
>+        (WebCore::Display::Box::moveHorizontally):
>+        (WebCore::Display::Box::moveVertically):
>+        (WebCore::Display::Box::Rect::setWidth):
>+        (WebCore::Display::Box::Rect::setHeight):
>+        (WebCore::Display::Box::Rect::moveHorizontally):
>+        (WebCore::Display::Box::Rect::moveVertically):
>+        * layout/layouttree/LayoutBox.cpp:
>+        (WebCore::Layout::Box::isPaddingApplicable const):
>+        * layout/layouttree/LayoutBox.h:
>+        * layout/layouttree/LayoutTreeBuilder.cpp:
>+        (WebCore::Layout::TreeBuilder::createSubTree):
>+
> 2018-06-12  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Add vertical margin computation for inflow non-replaced box and for the (10.6.6) complicated cases.
>diff --git a/Source/WebCore/layout/FormattingContext.cpp b/Source/WebCore/layout/FormattingContext.cpp
>index 73a504a2f956461bcb9fed969dd3e2a966ed1264..0d9ea3af88620b03979e3ca0e3bf2da5c3ebb353 100644
>--- a/Source/WebCore/layout/FormattingContext.cpp
>+++ b/Source/WebCore/layout/FormattingContext.cpp
>@@ -53,6 +53,7 @@ void FormattingContext::computeFloatingHeightAndMargin(LayoutContext& layoutCont
> {
>     auto heightAndMargin = Geometry::floatingHeightAndMargin(layoutContext, layoutBox);
>     displayBox.setHeight(heightAndMargin.height);
>+    displayBox.moveVertically(heightAndMargin.margin.top);
>     displayBox.setVerticalMargin(heightAndMargin.margin);
> }
> 
>@@ -60,6 +61,7 @@ void FormattingContext::computeFloatingWidthAndMargin(LayoutContext& layoutConte
> {
>     auto widthAndMargin = Geometry::floatingWidthAndMargin(layoutContext, layoutBox);
>     displayBox.setWidth(widthAndMargin.width);
>+    displayBox.moveHorizontally(widthAndMargin.margin.left);
>     displayBox.setHorizontalMargin(widthAndMargin.margin);
> }
> 
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>index 3c42f4cead96320de9c1ccfb2cd1a515e080e653..7e03cd672abd10e4ed52fb744b05973536cb812d 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContext.cpp
>@@ -73,9 +73,9 @@ void BlockFormattingContext::layout(LayoutContext& layoutContext, FormattingStat
>             auto& layoutBox = layoutPair.layoutBox;
>             auto& displayBox = layoutPair.displayBox;
>             
>+            computeStaticPosition(layoutContext, layoutBox, displayBox);
>             computeBorderAndPadding(layoutContext, layoutBox, displayBox);
>             computeWidthAndMargin(layoutContext, layoutBox, displayBox);
>-            computeStaticPosition(layoutContext, layoutBox, displayBox);
>             if (layoutBox.establishesFormattingContext()) {
>                 auto formattingContext = layoutContext.formattingContext(layoutBox);
>                 formattingContext->layout(layoutContext, layoutContext.establishedFormattingState(layoutBox, *formattingContext));
>@@ -164,6 +164,7 @@ void BlockFormattingContext::computeInFlowHeightAndMargin(LayoutContext& layoutC
> {
>     auto heightAndMargin = Geometry::inFlowHeightAndMargin(layoutContext, layoutBox);
>     displayBox.setHeight(heightAndMargin.height);
>+    displayBox.moveVertically(heightAndMargin.margin.top);
>     displayBox.setVerticalMargin(heightAndMargin.margin);
> }
> 
>@@ -171,6 +172,7 @@ void BlockFormattingContext::computeInFlowWidthAndMargin(LayoutContext& layoutCo
> {
>     auto widthAndMargin = Geometry::inFlowWidthAndMargin(layoutContext, layoutBox);
>     displayBox.setWidth(widthAndMargin.width);
>+    displayBox.moveHorizontally(widthAndMargin.margin.left);
>     displayBox.setHorizontalMargin(widthAndMargin.margin);
> }
> 
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>index 77d4e7a45580676172eb66942d613b69ad211e42..1bc945a6995a3f9f2f0c184ca12078e91a7db7fc 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>@@ -40,7 +40,7 @@ static bool isStretchedToViewport(const LayoutContext& layoutContext, const Box&
>     if (!layoutContext.inQuirksMode())
>         return false;
> 
>-    if (!layoutBox.isDocumentBox() || !layoutBox.isBodyBox())
>+    if (!layoutBox.isDocumentBox() && !layoutBox.isBodyBox())
>         return false;
> 
>     return layoutBox.style().logicalHeight().isAuto();
>@@ -114,8 +114,13 @@ FormattingContext::Geometry::HeightAndMargin BlockFormattingContext::Geometry::i
> 
>     if (!isStretchedToViewport(layoutContext, layoutBox))
>         return { height, { marginTop, marginBottom } };
>+
>     auto initialContainingBlockHeight = layoutContext.displayBoxForLayoutBox(initialContainingBlock(layoutBox))->contentBox().height();
>-    return { std::max(height, initialContainingBlockHeight), { marginTop, marginBottom } };
>+    // Stretch but never overstretch with the margins.
>+    if (height + marginTop + marginBottom < initialContainingBlockHeight)
>+        height = initialContainingBlockHeight - marginTop - marginBottom;
>+
>+    return { height, { marginTop, marginBottom } };
> }
> 
> FormattingContext::Geometry::WidthAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox,
>@@ -220,8 +225,14 @@ FormattingContext::Geometry::WidthAndMargin BlockFormattingContext::Geometry::in
>     auto computedWidthAndMarginValue = compute();
>     if (!isStretchedToViewport(layoutContext, layoutBox))
>         return computedWidthAndMarginValue;
>+
>     auto initialContainingBlockWidth = layoutContext.displayBoxForLayoutBox(initialContainingBlock(layoutBox))->contentBox().width();
>-    return FormattingContext::Geometry::WidthAndMargin { std::max(computedWidthAndMarginValue.width, initialContainingBlockWidth), { computedWidthAndMarginValue.margin } };
>+    auto horizontalMargins = computedWidthAndMarginValue.margin.left + computedWidthAndMarginValue.margin.right; 
>+    // Stretch but never overstretch with the margins.
>+    if (computedWidthAndMarginValue.width + horizontalMargins < initialContainingBlockWidth)
>+        computedWidthAndMarginValue.width = initialContainingBlockWidth - horizontalMargins;
>+
>+    return { computedWidthAndMarginValue.width, computedWidthAndMarginValue.margin };
> }
> 
> FormattingContext::Geometry::WidthAndMargin BlockFormattingContext::Geometry::inFlowReplacedWidthAndMargin(LayoutContext& layoutContext, const Box& layoutBox)
>@@ -256,10 +267,7 @@ LayoutPoint BlockFormattingContext::Geometry::staticPosition(LayoutContext& layo
>         auto& previousInFlowDisplayBox = *layoutContext.displayBoxForLayoutBox(*previousInFlowSibling);
>         top = previousInFlowDisplayBox.bottom() + previousInFlowDisplayBox.marginBottom();
>     }
>-    auto& displayBox = *layoutContext.displayBoxForLayoutBox(layoutBox);
>-    LayoutPoint topLeft = { top, left };
>-    topLeft.moveBy({ displayBox.marginLeft(), displayBox.marginTop() });
>-    return topLeft;
>+    return { top, left };
> }
> 
> LayoutPoint BlockFormattingContext::Geometry::inFlowPositionedPosition(LayoutContext& layoutContext, const Box& layoutBox)
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.h b/Source/WebCore/layout/displaytree/DisplayBox.h
>index f717aa5c47e9ad8b80bc92340e2d69f87dc1baf6..eacd26f4f044ceb6a317667e0993332ce1251dae 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.h
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.h
>@@ -80,6 +80,9 @@ public:
>         void shiftTopTo(LayoutUnit);
>         void shiftBottomTo(LayoutUnit);
> 
>+        void moveHorizontally(LayoutUnit);
>+        void moveVertically(LayoutUnit);
>+
>         void expand(LayoutUnit, LayoutUnit);
> 
>         Rect clone() const;
>@@ -156,6 +159,8 @@ private:
>     void setTopLeft(const LayoutPoint& topLeft) { m_rect.setTopLeft(topLeft); }
>     void setTop(LayoutUnit top) { m_rect.setTop(top); }
>     void setLeft(LayoutUnit left) { m_rect.setLeft(left); }
>+    void moveHorizontally(LayoutUnit offset) { m_rect.moveHorizontally(offset); }
>+    void moveVertically(LayoutUnit offset) { m_rect.moveVertically(offset); }
>     void setWidth(LayoutUnit width) { m_rect.setWidth(width); }
>     void setHeight(LayoutUnit height) { m_rect.setHeight(height); }
>     void setSize(const LayoutSize& size) { m_rect.setSize(size); }
>@@ -317,7 +322,6 @@ inline void Box::Rect::setWidth(LayoutUnit width)
> #if !ASSERT_DISABLED
>     m_hasValidWidth = true;
> #endif
>-    ASSERT(m_hasValidLeft);
>     m_rect.setWidth(width);
> }
> 
>@@ -326,7 +330,6 @@ inline void Box::Rect::setHeight(LayoutUnit height)
> #if !ASSERT_DISABLED
>     m_hasValidHeight = true;
> #endif
>-    ASSERT(m_hasValidTop);
>     m_rect.setHeight(height);
> }
> 
>@@ -362,6 +365,18 @@ inline void Box::Rect::shiftBottomTo(LayoutUnit bottom)
>     m_rect.shiftMaxYEdgeTo(bottom);
> }
> 
>+inline void Box::Rect::moveHorizontally(LayoutUnit offset)
>+{
>+    ASSERT(m_hasValidLeft);
>+    m_rect.move(offset, { });
>+}
>+
>+inline void Box::Rect::moveVertically(LayoutUnit offset)
>+{
>+    ASSERT(m_hasValidTop);
>+    m_rect.move({ }, offset);
>+}
>+
> inline void Box::Rect::expand(LayoutUnit width, LayoutUnit height)
> {
>     ASSERT(hasValidGeometry());
>diff --git a/Source/WebCore/layout/layouttree/LayoutBox.cpp b/Source/WebCore/layout/layouttree/LayoutBox.cpp
>index defad2c509e3cc6a320c18ac1e91d2bc4261319c..71c09c26511ed894857c979f2e77bf22fc2bf277 100644
>--- a/Source/WebCore/layout/layouttree/LayoutBox.cpp
>+++ b/Source/WebCore/layout/layouttree/LayoutBox.cpp
>@@ -230,8 +230,9 @@ bool Box::isPaddingApplicable() const
> {
>     // 8.4 Padding properties:
>     // Applies to: all elements except table-row-group, table-header-group, table-footer-group, table-row, table-column-group and table-column
>-    if (!m_elementAttributes)
>+    if (isAnonymous())
>         return false;
>+
>     auto elementType = m_elementAttributes.value().elementType;
>     return elementType != ElementType::TableRowGroup
>         && elementType != ElementType::TableHeaderGroup
>diff --git a/Source/WebCore/layout/layouttree/LayoutBox.h b/Source/WebCore/layout/layouttree/LayoutBox.h
>index 9db2060162207896ecaea20c40a552e173b75e8c..718ae85446ce5b7d6e3581ed444cd358e88230b4 100644
>--- a/Source/WebCore/layout/layouttree/LayoutBox.h
>+++ b/Source/WebCore/layout/layouttree/LayoutBox.h
>@@ -106,7 +106,8 @@ protected:
>         TableColumnGroup,
>         TableRowGroup,
>         TableHeaderGroup,
>-        TableFooterGroup 
>+        TableFooterGroup,
>+        GenericElement
>     };
> 
>     struct ElementAttributes {
>diff --git a/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp b/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp
>index d8898ef76e8eac4dce0847592a978668843fe28f..ae34a30215d675a45477b9ec2f59ddf6f4004881 100644
>--- a/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp
>+++ b/Source/WebCore/layout/layouttree/LayoutTreeBuilder.cpp
>@@ -72,7 +72,10 @@ void TreeBuilder::createSubTree(const RenderElement& rootRenderer, Container& ro
>                 return Box::ElementAttributes { Box::ElementType::TableHeaderGroup };
>             if (element->hasTagName(HTMLNames::tfootTag))
>                 return Box::ElementAttributes { Box::ElementType::TableFooterGroup };
>-        } 
>+            if (element->hasTagName(HTMLNames::tfootTag))
>+                return Box::ElementAttributes { Box::ElementType::TableFooterGroup };
>+            return Box::ElementAttributes { Box::ElementType::GenericElement };
>+        }
>         return std::nullopt;
>     };
>

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 186616: 342726