Attachment 343768 Details for Bug 187124 – Patch

[patch] Patch

bug-187124-20180627164213.patch (text/plain), 7.98 KB, created by zalan on 2018-06-27 16:42:14 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-06-27 16:42:14 PDT

Size: 7.98 KB

patch

obsolete

>Subversion Revision: 233275
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index ad5b4aebafcaef04bb0204eac6a97013267c3351..d2e9d2b86d40385f347a58b00e9ac144f8a61a85 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,13 @@
>+2018-06-27  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] Align inFlowNonReplacedWidthAndMargin() style with the rest of the compute functions.
>+        https://bugs.webkit.org/show_bug.cgi?id=187124
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin):
>+
> 2018-06-27  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Do not collapse margin with the parent when element has border/padding.
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>index 37e8d2a185ed44c8aabbf969555f28bcf6f5ffbd..442131aeba50f5e5626a0d3c90064f67d8cded0d 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>@@ -138,7 +138,9 @@ FormattingContext::Geometry::WidthAndMargin BlockFormattingContext::Geometry::in
>     ASSERT(layoutBox.isInFlow() && !layoutBox.replaced());
> 
>     auto compute = [&]() {
>+
>         // 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
>         //
>@@ -159,76 +161,61 @@ FormattingContext::Geometry::WidthAndMargin BlockFormattingContext::Geometry::in
>         //    edges of the containing block.
> 
>         auto& style = layoutBox.style();
>-        auto width = precomputedWidth ? Length { precomputedWidth.value(), Fixed } : style.logicalWidth();
>         auto* containingBlock = layoutBox.containingBlock();
>         auto containingBlockWidth = layoutContext.displayBoxForLayoutBox(*containingBlock)->contentBoxWidth();
>         auto& displayBox = *layoutContext.displayBoxForLayoutBox(layoutBox);
> 
>-        LayoutUnit computedWidthValue;
>-        std::optional<LayoutUnit> computedMarginLeftValue;
>-        std::optional<LayoutUnit> computedMarginRightValue;
>-
>-        auto marginLeft = style.marginLeft();
>-        if (!marginLeft.isAuto())
>-            computedMarginLeftValue = valueForLength(marginLeft, containingBlockWidth);
>-
>-        auto marginRight = style.marginRight();
>-        if (!marginRight.isAuto())
>-            computedMarginRightValue = valueForLength(marginRight, containingBlockWidth);
>-
>+        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 borderLeft = displayBox.borderLeft();
>         auto borderRight = displayBox.borderRight();
>         auto paddingLeft = displayBox.paddingLeft();
>         auto paddingRight = displayBox.paddingRight();
> 
>         // #1
>-        if (!width.isAuto()) {
>-            computedWidthValue = valueForLength(width, containingBlockWidth);
>-            auto horizontalSpaceForMargin = containingBlockWidth - (computedMarginLeftValue.value_or(0) + borderLeft + paddingLeft 
>-                + computedWidthValue + paddingRight + borderRight + computedMarginRightValue.value_or(0));
>-
>+        if (width) {
>+            auto horizontalSpaceForMargin = containingBlockWidth - (marginLeft.value_or(0) + borderLeft + paddingLeft + *width + paddingRight + borderRight + marginRight.value_or(0));
>             if (horizontalSpaceForMargin < 0) {
>-                if (!computedMarginLeftValue)
>-                    computedMarginLeftValue = LayoutUnit(0);
>-                if (!computedMarginRightValue)
>-                    computedMarginRightValue = LayoutUnit(0);
>+                marginLeft = marginLeft.value_or(0);
>+                marginRight = marginRight.value_or(0);
>             }
>         }
> 
>         // #2
>-        if (!width.isAuto() && !marginLeft.isAuto() && !marginRight.isAuto()) {
>-            ASSERT(computedMarginLeftValue);
>-            ASSERT(computedMarginRightValue);
>-
>+        if (width && marginLeft && marginRight) {
>             if (containingBlock->style().isLeftToRightDirection())
>-                computedMarginRightValue = containingBlockWidth - (*computedMarginLeftValue + borderLeft + paddingLeft  + computedWidthValue + paddingRight + borderRight);
>+                marginRight = containingBlockWidth - (*marginLeft + borderLeft + paddingLeft  + *width + paddingRight + borderRight);
>             else
>-                computedMarginLeftValue = containingBlockWidth - (borderLeft + paddingLeft  + computedWidthValue + paddingRight + borderRight + *computedMarginRightValue);
>+                marginLeft = containingBlockWidth - (borderLeft + paddingLeft + *width + paddingRight + borderRight + *marginRight);
>         }
> 
>         // #3
>-        if (!computedMarginLeftValue && !marginRight.isAuto() && !width.isAuto()) {
>-            ASSERT(computedMarginRightValue);
>-            computedMarginLeftValue = containingBlockWidth - (borderLeft + paddingLeft  + computedWidthValue + paddingRight + borderRight + *computedMarginRightValue);
>-        } else if (!computedMarginRightValue && !marginLeft.isAuto() && !width.isAuto()) {
>-            ASSERT(computedMarginLeftValue);
>-            computedMarginRightValue = containingBlockWidth - (*computedMarginLeftValue + borderLeft + paddingLeft  + computedWidthValue + paddingRight + borderRight);
>-        }
>+        if (!marginLeft && width && marginRight)
>+            marginLeft = containingBlockWidth - (borderLeft + paddingLeft  + *width + paddingRight + borderRight + *marginRight);
>+        else if (marginLeft && !width && marginRight)
>+            width = containingBlockWidth - (*marginLeft + borderLeft + paddingLeft + paddingRight + borderRight + *marginRight);
>+        else if (marginLeft && width && !marginRight)
>+            marginRight = containingBlockWidth - (*marginLeft + borderLeft + paddingLeft + *width + paddingRight + borderRight);
> 
>         // #4
>-        if (width.isAuto())
>-            computedWidthValue = containingBlockWidth - (computedMarginLeftValue.value_or(0) + borderLeft + paddingLeft + paddingRight + borderRight + computedMarginRightValue.value_or(0));
>+        if (!width) {
>+            marginLeft = marginLeft.value_or(0);
>+            marginRight = marginRight.value_or(0);
>+            width = containingBlockWidth - (*marginLeft + borderLeft + paddingLeft + paddingRight + borderRight + *marginRight);
>+        }
> 
>         // #5
>-        if (!computedMarginLeftValue && !computedMarginRightValue) {
>-            auto horizontalSpaceForMargin = containingBlockWidth - (borderLeft + paddingLeft  + computedWidthValue + paddingRight + borderRight);
>-            computedMarginLeftValue = computedMarginRightValue = horizontalSpaceForMargin / 2;
>+        if (!marginLeft && !marginRight) {
>+            auto horizontalSpaceForMargin = containingBlockWidth - (borderLeft + paddingLeft  + *width + paddingRight + borderRight);
>+            marginLeft = marginRight = horizontalSpaceForMargin / 2;
>         }
> 
>-        ASSERT(computedMarginLeftValue);
>-        ASSERT(computedMarginRightValue);
>+        ASSERT(width);
>+        ASSERT(marginLeft);
>+        ASSERT(marginRight);
> 
>-        return FormattingContext::Geometry::WidthAndMargin { computedWidthValue, { *computedMarginLeftValue, *computedMarginRightValue } };
>+        return FormattingContext::Geometry::WidthAndMargin { *width, { *marginLeft, *marginRight } };
>     };
> 
>     auto widthAndMargin = compute();

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 187124: 343768