Attachment 341573 Details for Bug 186083 – Patch

[patch] Patch

bug-186083-20180530074732.patch (text/plain), 19.05 KB, created by zalan on 2018-05-30 07:47:33 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-05-30 07:47:33 PDT

Size: 19.05 KB

patch

obsolete

>Subversion Revision: 232284
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 488a7b6b2c7f01bc852e9847df6a6e0805ee1282..e90af27890605a03d3e7b3dd4dd783f287b815b0 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2018-05-29  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] Miscellaneous fixes to get closer to geometry correctness
>+        https://bugs.webkit.org/show_bug.cgi?id=186083
>+
>+        Reviewed by Antti Koivisto.
>+
>+        * layout/FormattingContextGeometry.cpp:
>+        (WebCore::Layout::FormattingContext::Geometry::computedBorder):
>+        * layout/LayoutContext.cpp:
>+        (WebCore::Layout::LayoutContext::initializeRoot):
>+        * layout/Verification.cpp:
>+        (WebCore::Layout::outputMismatchingBoxInformationIfNeeded):
>+        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
>+        (WebCore::Layout::isStretchedToViewport):
>+        (WebCore::Layout::initialContainingBlock):
>+        (WebCore::Layout::computedInFlowNonReplacedComputedHeight):
>+        (WebCore::Layout::inFlowNonReplacedComputedWidth):
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedHeight): lambda should capture the specification part. 
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedWidth):
>+        * layout/displaytree/DisplayBox.cpp:
>+        (WebCore::Display::Box::marginBox const):
>+        (WebCore::Display::Box::paddingBox const):
>+        (WebCore::Display::Box::contentBox const):
>+        * layout/layouttree/LayoutBox.cpp:
>+        (WebCore::Layout::Box::isDocumentBox const):
>+        (WebCore::Layout::Box::isBodyBox const):
>+        * layout/layouttree/LayoutBox.h:
>+        * rendering/style/BorderValue.h: ignore border-width when type is hidden or none. 
>+        (WebCore::BorderValue::boxModelWidth const):
>+
> 2018-05-29  Youenn Fablet  <youenn@apple.com>
> 
>         Add a consistency check between URL and CFURL
>diff --git a/Source/WebCore/layout/FormattingContextGeometry.cpp b/Source/WebCore/layout/FormattingContextGeometry.cpp
>index 8c74ec541817ed09883a06f0792d6c7b3ca73093..789ff1a92610414595c41f53b14535a468ff53ba 100644
>--- a/Source/WebCore/layout/FormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/FormattingContextGeometry.cpp
>@@ -529,10 +529,10 @@ Display::Box::Edges FormattingContext::Geometry::computedBorder(LayoutContext&,
> {
>     auto& style = layoutBox.style();
>     return {
>-        style.borderTop().width(),
>-        style.borderLeft().width(),
>-        style.borderBottom().width(),
>-        style.borderRight().width()
>+        style.borderTop().boxModelWidth(),
>+        style.borderLeft().boxModelWidth(),
>+        style.borderBottom().boxModelWidth(),
>+        style.borderRight().boxModelWidth()
>     };
> }
> 
>diff --git a/Source/WebCore/layout/LayoutContext.cpp b/Source/WebCore/layout/LayoutContext.cpp
>index 8f28e7ab8df71e8891475b125e6a1744492b3858..e015b5197ae31ffb9c9b919931bfa22866fea922 100644
>--- a/Source/WebCore/layout/LayoutContext.cpp
>+++ b/Source/WebCore/layout/LayoutContext.cpp
>@@ -61,12 +61,12 @@ void LayoutContext::initializeRoot(const Container& root, const LayoutSize& cont
>     displayBox.setMargin({ });
> 
>     auto& style = root.style();
>-    // FIXME: m_root could very well be a formatting context root with ancestors and resolvable border and padding (as opposed to the topmost root)  
>+    // 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.setBorder({
>-        style.borderTop().width(),
>-        style.borderLeft().width(),
>-        style.borderBottom().width(),
>-        style.borderRight().width()
>+        style.borderTop().boxModelWidth(),
>+        style.borderLeft().boxModelWidth(),
>+        style.borderBottom().boxModelWidth(),
>+        style.borderRight().boxModelWidth()
>     });
> 
>     displayBox.setPadding({
>diff --git a/Source/WebCore/layout/Verification.cpp b/Source/WebCore/layout/Verification.cpp
>index a7af7311631574b8f47788ce9c71906691e50540..9ab808f1b8de556a0fce0fff55fe16493ab2ee0b 100644
>--- a/Source/WebCore/layout/Verification.cpp
>+++ b/Source/WebCore/layout/Verification.cpp
>@@ -68,7 +68,7 @@ static void outputMismatchingBoxInformationIfNeeded(TextStream& stream, const La
>     if (renderer.paddingBoxRect() != displayBox->paddingBox())
>         outputRect("paddingBox", renderer.paddingBoxRect(), displayBox->paddingBox());
> 
>-    if (renderer.marginBoxRect() != displayBox->contentBox())
>+    if (renderer.contentBoxRect() != displayBox->contentBox())
>         outputRect("contentBox", renderer.contentBoxRect(), displayBox->contentBox());
> 
>     stream.nextLine();
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>index 2531e903639c844daf5fbaa4502cddcfa7a21a6a..9e09f9f824843f7495237b0eead56e5fcbe2793b 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>@@ -33,87 +33,124 @@
> namespace WebCore {
> namespace Layout {
> 
>-LayoutUnit BlockFormattingContext::Geometry::inFlowNonReplacedHeight(LayoutContext& layoutContext, const Box& layoutBox)
>+static bool isStretchedToViewport(const Box& layoutBox)
> {
>-    ASSERT(layoutBox.isInFlow() && !layoutBox.replaced());
>+    ASSERT(layoutBox.isInFlow());
>+    // In quirks mode, body and html stretch to the viewport.
>+    // if (!layoutBox.document().inQuirksMode())
>+    //    return false;
> 
>-    // https://www.w3.org/TR/CSS22/visudet.html
>-    // If 'height' is 'auto', the height depends on whether the element has any block-level children and whether it has padding or borders:
>-    // The element's height is the distance from its top content edge to the first applicable of the following:
>-    // 1. the bottom edge of the last line box, if the box establishes a inline formatting context with one or more lines
>-    // 2. the bottom edge of the bottom (possibly collapsed) margin of its last in-flow child, if the child's bottom margin
>-    //    does not collapse with the element's bottom margin
>-    // 3. the bottom border edge of the last in-flow child whose top margin doesn't collapse with the element's bottom margin
>-    // 4. zero, otherwise
>-    // Only children in the normal flow are taken into account (i.e., floating boxes and absolutely positioned boxes are ignored,
>-    // and relatively positioned boxes are considered without their offset). Note that the child box may be an anonymous block box.
>-    if (!layoutBox.style().logicalHeight().isAuto()) {
>-        // FIXME: Only fixed values yet.
>-        return layoutBox.style().logicalHeight().value();
>-    }
>+    if (!layoutBox.isDocumentBox() || !layoutBox.isBodyBox())
>+        return false;
> 
>-    if (!is<Container>(layoutBox) || !downcast<Container>(layoutBox).hasInFlowChild())
>-        return 0;
>+    return layoutBox.style().logicalHeight().isAuto();
>+}
> 
>-    // 1. the bottom edge of the last line box, if the box establishes a inline formatting context with one or more lines
>-    if (layoutBox.establishesInlineFormattingContext()) {
>-        // height = lastLineBox().bottom();
>-        return 0;
>-    }
>+static const Container& initialContainingBlock(const Box& layoutBox)
>+{
>+    auto* containingBlock = layoutBox.containingBlock();
>+    while (containingBlock->containingBlock())
>+        containingBlock = containingBlock->containingBlock();
>+    return *containingBlock;
>+}
> 
>-    // 2. the bottom edge of the bottom (possibly collapsed) margin of its last in-flow child, if the child's bottom margin...
>-    auto* lastInFlowChild = downcast<Container>(layoutBox).lastInFlowChild();
>-    ASSERT(lastInFlowChild);
>-    if (!BlockFormattingContext::MarginCollapse::isMarginBottomCollapsedWithParent(*lastInFlowChild)) {
>-        auto* lastInFlowDisplayBox = layoutContext.displayBoxForLayoutBox(*lastInFlowChild);
>-        ASSERT(lastInFlowDisplayBox);
>-        return lastInFlowDisplayBox->bottom() + lastInFlowDisplayBox->marginBottom();
>-    }
>+LayoutUnit BlockFormattingContext::Geometry::inFlowNonReplacedHeight(LayoutContext& layoutContext, const Box& layoutBox)
>+{
>+    ASSERT(layoutBox.isInFlow() && !layoutBox.replaced());
> 
>-    // 3. the bottom border edge of the last in-flow child whose top margin doesn't collapse with the element's bottom margin
>-    auto* inFlowChild = lastInFlowChild;
>-    while (inFlowChild && BlockFormattingContext::MarginCollapse::isMarginTopCollapsedWithParentMarginBottom(*inFlowChild))
>-        inFlowChild = inFlowChild->previousInFlowSibling();
>-    if (inFlowChild) {
>-        auto* inFlowDisplayBox = layoutContext.displayBoxForLayoutBox(*inFlowChild);
>-        ASSERT(inFlowDisplayBox);
>-        return inFlowDisplayBox->top() + inFlowDisplayBox->borderBox().height();
>-    }
>+    auto computeHeight = [&]() -> LayoutUnit {
>+        // https://www.w3.org/TR/CSS22/visudet.html
>+        // If 'height' is 'auto', the height depends on whether the element has any block-level children and whether it has padding or borders:
>+        // The element's height is the distance from its top content edge to the first applicable of the following:
>+        // 1. the bottom edge of the last line box, if the box establishes a inline formatting context with one or more lines
>+        // 2. the bottom edge of the bottom (possibly collapsed) margin of its last in-flow child, if the child's bottom margin
>+        //    does not collapse with the element's bottom margin
>+        // 3. the bottom border edge of the last in-flow child whose top margin doesn't collapse with the element's bottom margin
>+        // 4. zero, otherwise
>+        // Only children in the normal flow are taken into account (i.e., floating boxes and absolutely positioned boxes are ignored,
>+        // and relatively positioned boxes are considered without their offset). Note that the child box may be an anonymous block box.
>+        if (!layoutBox.style().logicalHeight().isAuto()) {
>+            // FIXME: Only fixed values yet.
>+            return layoutBox.style().logicalHeight().value();
>+        }
>+
>+        if (!is<Container>(layoutBox) || !downcast<Container>(layoutBox).hasInFlowChild())
>+            return 0;
>+
>+        // 1. the bottom edge of the last line box, if the box establishes a inline formatting context with one or more lines
>+        if (layoutBox.establishesInlineFormattingContext()) {
>+            // height = lastLineBox().bottom();
>+            return 0;
>+        }
>+
>+        // 2. the bottom edge of the bottom (possibly collapsed) margin of its last in-flow child, if the child's bottom margin...
>+        auto* lastInFlowChild = downcast<Container>(layoutBox).lastInFlowChild();
>+        ASSERT(lastInFlowChild);
>+        if (!BlockFormattingContext::MarginCollapse::isMarginBottomCollapsedWithParent(*lastInFlowChild)) {
>+            auto* lastInFlowDisplayBox = layoutContext.displayBoxForLayoutBox(*lastInFlowChild);
>+            ASSERT(lastInFlowDisplayBox);
>+            return lastInFlowDisplayBox->bottom() + lastInFlowDisplayBox->marginBottom();
>+        }
>+
>+        // 3. the bottom border edge of the last in-flow child whose top margin doesn't collapse with the element's bottom margin
>+        auto* inFlowChild = lastInFlowChild;
>+        while (inFlowChild && BlockFormattingContext::MarginCollapse::isMarginTopCollapsedWithParentMarginBottom(*inFlowChild))
>+            inFlowChild = inFlowChild->previousInFlowSibling();
>+        if (inFlowChild) {
>+            auto* inFlowDisplayBox = layoutContext.displayBoxForLayoutBox(*inFlowChild);
>+            ASSERT(inFlowDisplayBox);
>+            return inFlowDisplayBox->top() + inFlowDisplayBox->borderBox().height();
>+        }
>+
>+        // 4. zero, otherwise
>+        return 0;
>+    };
> 
>-    // 4. zero, otherwise
>-    return 0;
>+    auto computedHeight = computeHeight();
>+    if (!isStretchedToViewport(layoutBox))
>+        return computedHeight;
>+    auto initialContainingBlockHeight = layoutContext.displayBoxForLayoutBox(initialContainingBlock(layoutBox))->contentBox().height();
>+    return std::max(computedHeight, initialContainingBlockHeight);
> }
> 
> LayoutUnit BlockFormattingContext::Geometry::inFlowNonReplacedWidth(LayoutContext& layoutContext, const Box& layoutBox)
> {
>     ASSERT(layoutBox.isInFlow() && !layoutBox.replaced());
> 
>-    // 10.3.3 Block-level, non-replaced elements in normal flow
>-    // The following constraints must hold among the used values of the other properties:
>-    // 'margin-left' + 'border-left-width' + 'padding-left' + 'width' + 'padding-right' + 'border-right-width' + 'margin-right' = width of containing block
>+    auto computeWidth = [&]() {
>+        // 10.3.3 Block-level, non-replaced elements in normal flow
>+        // The following constraints must hold among the used values of the other properties:
>+        // 'margin-left' + 'border-left-width' + 'padding-left' + 'width' + 'padding-right' + 'border-right-width' + 'margin-right' = width of containing block
> 
>-    // If 'width' is set to 'auto', any other 'auto' values become '0' and 'width' follows from the resulting equality.
>-    auto& style = layoutBox.style();
>-    auto containingBlockWidth = layoutContext.displayBoxForLayoutBox(*layoutBox.containingBlock())->width();
>+        // If 'width' is set to 'auto', any other 'auto' values become '0' and 'width' follows from the resulting equality.
>+        auto& style = layoutBox.style();
>+        auto containingBlockWidth = layoutContext.displayBoxForLayoutBox(*layoutBox.containingBlock())->width();
>+
>+        LayoutUnit computedWidthValue;
>+        auto width = style.logicalWidth();
>+        if (width.isAuto()) {
>+            auto& displayBox = *layoutContext.displayBoxForLayoutBox(layoutBox);
>+            auto marginLeft = displayBox.marginLeft();
>+            auto marginRight = displayBox.marginRight();
> 
>-    LayoutUnit computedWidthValue;
>-    auto width = style.logicalWidth();
>-    if (width.isAuto()) {
>-        auto& displayBox = *layoutContext.displayBoxForLayoutBox(layoutBox);
>-        auto marginLeft = displayBox.marginLeft();
>-        auto marginRight = displayBox.marginRight();
>+            auto paddingLeft = displayBox.paddingLeft();
>+            auto paddingRight = displayBox.paddingRight();
> 
>-        auto paddingLeft = displayBox.paddingLeft();
>-        auto paddingRight = displayBox.paddingRight();
>+            auto borderLeft = displayBox.borderLeft();
>+            auto borderRight = displayBox.borderRight();
> 
>-        auto borderLeft = displayBox.borderLeft();
>-        auto borderRight = displayBox.borderRight();
>+            computedWidthValue = containingBlockWidth - (marginLeft + borderLeft + paddingLeft + paddingRight + borderRight + marginRight);
>+        } else
>+            computedWidthValue = valueForLength(width, containingBlockWidth);
> 
>-        computedWidthValue = containingBlockWidth - (marginLeft + borderLeft + paddingLeft + paddingRight + borderRight + marginRight);
>-    } else
>-        computedWidthValue = valueForLength(width, containingBlockWidth);
>+        return computedWidthValue;
>+    };
> 
>-    return computedWidthValue;
>+    auto computedWidth = computeWidth();
>+    if (!isStretchedToViewport(layoutBox))
>+        return computedWidth;
>+    auto initialContainingBlockWidth = layoutContext.displayBoxForLayoutBox(initialContainingBlock(layoutBox))->contentBox().width();
>+    return std::max(computedWidth, initialContainingBlockWidth);
> }
> 
> LayoutPoint 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 d7d55847efcd85bb0154f496d1a01b5ce971b94a..d0c8b9e40f161f62a97ae17f0a55e3ce2aa57abd 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.cpp
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.cpp
>@@ -56,7 +56,7 @@ Box::Rect Box::marginBox() const
>     auto marginBox = borderBox();
> 
>     marginBox.shiftLeftTo(marginBox.left() + m_margin.left);
>-    marginBox.shiftBottomTo(marginBox.top() + m_margin.top);
>+    marginBox.shiftTopTo(marginBox.top() + m_margin.top);
>     marginBox.shiftRightTo(marginBox.right() - m_margin.right);
>     marginBox.shiftBottomTo(marginBox.bottom() - m_margin.bottom);
> 
>@@ -85,7 +85,7 @@ Box::Rect Box::paddingBox() const
> 
>     paddingBox.shiftLeftTo(paddingBox.left() + m_border.left);
>     paddingBox.shiftTopTo(paddingBox.top() + m_border.top);
>-    paddingBox.shiftRightTo(paddingBox.left() - m_border.right);
>+    paddingBox.shiftRightTo(paddingBox.right() - m_border.right);
>     paddingBox.shiftBottomTo(paddingBox.bottom() - m_border.bottom);
> 
>     return paddingBox;
>@@ -105,8 +105,8 @@ Box::Rect Box::contentBox() const
> 
>     contentBox.shiftLeftTo(contentBox.left() + m_padding.left);
>     contentBox.shiftTopTo(contentBox.top() + m_padding.top);
>-    contentBox.shiftBottomTo(contentBox.bottom() - m_padding.bottom);
>     contentBox.shiftRightTo(contentBox.right() - m_padding.right);
>+    contentBox.shiftBottomTo(contentBox.bottom() - m_padding.bottom);
> 
>     return contentBox;
> }
>diff --git a/Source/WebCore/layout/layouttree/LayoutBox.cpp b/Source/WebCore/layout/layouttree/LayoutBox.cpp
>index 9669b9440eb24a3c12d44a15d4fa7f079500ce0e..540318a0e324fa3c38f1899f1ed1569f8c88e314 100644
>--- a/Source/WebCore/layout/layouttree/LayoutBox.cpp
>+++ b/Source/WebCore/layout/layouttree/LayoutBox.cpp
>@@ -181,6 +181,18 @@ bool Box::isInitialContainingBlock() const
>     return !parent();
> }
> 
>+bool Box::isDocumentBox() const
>+{
>+    // return document().documentElement() == &element();
>+    return false;
>+}
>+
>+bool Box::isBodyBox() const
>+{
>+    // return element().hasTagName(HTMLNames::bodyTag);
>+    return false;
>+}
>+
> const Box* Box::nextInFlowSibling() const
> {
>     if (auto* nextSibling = this->nextSibling()) {
>diff --git a/Source/WebCore/layout/layouttree/LayoutBox.h b/Source/WebCore/layout/layouttree/LayoutBox.h
>index 1e0b85ffde91368b6982ce116c82c13a1c29d21e..f98214c15334bb71aa3dc5979aeee932ab286a76 100644
>--- a/Source/WebCore/layout/layouttree/LayoutBox.h
>+++ b/Source/WebCore/layout/layouttree/LayoutBox.h
>@@ -74,6 +74,9 @@ public:
>     bool isBlockContainerBox() const;
>     bool isInitialContainingBlock() const;
> 
>+    bool isDocumentBox() const;
>+    bool isBodyBox() const;
>+
>     const Container* parent() const { return m_parent; }
>     const Box* nextSibling() const { return m_nextSibling; }
>     const Box* nextInFlowSibling() const;
>diff --git a/Source/WebCore/rendering/style/BorderValue.h b/Source/WebCore/rendering/style/BorderValue.h
>index bf849655aec99983d41917b0c1aa162e6f0db524..6f7f821ea626605e84c99560290c87b2e427f010 100644
>--- a/Source/WebCore/rendering/style/BorderValue.h
>+++ b/Source/WebCore/rendering/style/BorderValue.h
>@@ -72,6 +72,8 @@ public:
> 
>     float width() const { return m_width; }
>     BorderStyle style() const { return static_cast<BorderStyle>(m_style); }
>+    
>+    float boxModelWidth() const;
> 
> protected:
>     float m_width { 3 };
>@@ -83,4 +85,13 @@ protected:
>     unsigned m_isAuto : 1; // OutlineIsAuto
> };
> 
>+inline float BorderValue::boxModelWidth() const
>+{
>+    auto style = this->style();
>+    if (style == BorderStyle::None || style == BorderStyle::Hidden)
>+        return 0;
>+
>+    return width();
>+}
>+
> } // namespace WebCore

Actions: View | Formatted Diff | Diff

Attachments on bug 186083: 341569 | 341573 | 341574