Attachment 361493 Details for Bug 194428 – Patch

[patch] Patch

Patch.txt (text/plain), 10.94 KB, created by zalan on 2019-02-07 21:51:09 PST

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-02-07 21:51:09 PST

Size: 10.94 KB

patch

obsolete

>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 4841395f8fc..66b229f62d9 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,34 @@
>+2019-02-07  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC] The used containing block width value is optional
>+        https://bugs.webkit.org/show_bug.cgi?id=194428
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The preferred width codepath cannot provide a valid used containing block width value.
>+
>+        "The percentage is calculated with respect to the width of the generated box's containing block.
>+        If the containing block's width depends on this element's width, then the resulting layout is undefined in CSS 2.2."
>+
>+        Let's use 0 as used value for now.
>+
>+        * layout/FormattingContextGeometry.cpp:
>+        (WebCore::Layout::FormattingContext::Geometry::shrinkToFitWidth):
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGeometry):
>+        (WebCore::Layout::FormattingContext::Geometry::outOfFlowReplacedHorizontalGeometry):
>+        (WebCore::Layout::FormattingContext::Geometry::floatingNonReplacedWidthAndMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::floatingReplacedWidthAndMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::inlineReplacedWidthAndMargin):
>+        (WebCore::Layout::FormattingContext::Geometry::computedPadding):
>+        (WebCore::Layout::FormattingContext::Geometry::computedHorizontalMargin):
>+        * layout/LayoutUnits.h:
>+        (WebCore::Layout::UsedHorizontalValues::UsedHorizontalValues):
>+        * layout/blockformatting/BlockFormattingContextGeometry.cpp:
>+        (WebCore::Layout::BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin):
>+        * layout/inlineformatting/InlineFormattingContextGeometry.cpp:
>+        (WebCore::Layout::InlineFormattingContext::Geometry::inlineBlockWidthAndMargin):
>+        * page/FrameViewLayoutContext.cpp:
>+
> 2019-02-07  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC] Horizontal geometry compute functions should take the containing block's width as a used value
>diff --git a/Source/WebCore/layout/FormattingContextGeometry.cpp b/Source/WebCore/layout/FormattingContextGeometry.cpp
>index 809b017582b..afa0bc489d8 100644
>--- a/Source/WebCore/layout/FormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/FormattingContextGeometry.cpp
>@@ -246,7 +246,8 @@ LayoutUnit FormattingContext::Geometry::shrinkToFitWidth(LayoutState& layoutStat
>     // 'padding-left', 'padding-right', 'border-right-width', 'margin-right', and the widths of any relevant scroll bars.
> 
>     // Then the shrink-to-fit width is: min(max(preferred minimum width, available width), preferred width).
>-    auto availableWidth = usedValues.containingBlockWidth;
>+    ASSERT(usedValues.containingBlockWidth.hasValue());
>+    auto availableWidth = *usedValues.containingBlockWidth;
>     auto instrinsicWidthConstraints = layoutState.createFormattingContext(formattingRoot)->instrinsicWidthConstraints();
>     return std::min(std::max(instrinsicWidthConstraints.minimum, availableWidth), instrinsicWidthConstraints.maximum);
> }
>@@ -410,7 +411,7 @@ HorizontalGeometry FormattingContext::Geometry::outOfFlowNonReplacedHorizontalGe
>     auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
>     auto& containingBlock = *layoutBox.containingBlock();
>     auto& containingBlockDisplayBox = layoutState.displayBoxForLayoutBox(containingBlock);
>-    auto containingBlockWidth = usedValues.containingBlockWidth;
>+    auto containingBlockWidth = usedValues.containingBlockWidth.valueOr(0);
>     auto isLeftToRightDirection = containingBlock.style().isLeftToRightDirection();
>     
>     auto left = computedValueIfNotAuto(style.logicalLeft(), containingBlockWidth);
>@@ -635,7 +636,7 @@ HorizontalGeometry FormattingContext::Geometry::outOfFlowReplacedHorizontalGeome
>     auto& style = layoutBox.style();
>     auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
>     auto& containingBlock = *layoutBox.containingBlock();
>-    auto containingBlockWidth = usedValues.containingBlockWidth;
>+    auto containingBlockWidth = usedValues.containingBlockWidth.valueOr(0);
>     auto isLeftToRightDirection = containingBlock.style().isLeftToRightDirection();
> 
>     auto left = computedValueIfNotAuto(style.logicalLeft(), containingBlockWidth);
>@@ -766,7 +767,7 @@ WidthAndMargin FormattingContext::Geometry::floatingNonReplacedWidthAndMargin(La
>     // #1
>     auto usedHorizontallMargin = UsedHorizontalMargin { computedHorizontalMargin.start.valueOr(0), computedHorizontalMargin.end.valueOr(0) };
>     // #2
>-    auto width = computedValueIfNotAuto(usedValues.width ? Length { usedValues.width.value(), Fixed } : layoutBox.style().logicalWidth(), usedValues.containingBlockWidth);
>+    auto width = computedValueIfNotAuto(usedValues.width ? Length { usedValues.width.value(), Fixed } : layoutBox.style().logicalWidth(), usedValues.containingBlockWidth.valueOr(0));
>     if (!width)
>         width = shrinkToFitWidth(layoutState, layoutBox, usedValues);
> 
>@@ -795,7 +796,8 @@ WidthAndMargin FormattingContext::Geometry::floatingReplacedWidthAndMargin(const
>     auto computedHorizontalMargin = Geometry::computedHorizontalMargin(layoutBox, usedValues);
> 
>     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height][Margin] -> floating replaced -> redirected to inline replaced");
>-    return inlineReplacedWidthAndMargin(layoutState, layoutBox, UsedHorizontalValues { usedValues.containingBlockWidth, usedValues.width, UsedHorizontalMargin { computedHorizontalMargin.start.valueOr(0), computedHorizontalMargin.end.valueOr(0) } });
>+    return inlineReplacedWidthAndMargin(layoutState, layoutBox, UsedHorizontalValues { usedValues.containingBlockWidth.valueOr(0),
>+        usedValues.width, UsedHorizontalMargin { computedHorizontalMargin.start.valueOr(0), computedHorizontalMargin.end.valueOr(0) } });
> }
> 
> VerticalGeometry FormattingContext::Geometry::outOfFlowVerticalGeometry(const LayoutState& layoutState, const Box& layoutBox, UsedVerticalValues usedValues)
>@@ -903,7 +905,7 @@ WidthAndMargin FormattingContext::Geometry::inlineReplacedWidthAndMargin(const L
>     //    If 300px is too wide to fit the device, UAs should use the width of the largest rectangle that has a 2:1 ratio and fits the device instead.
> 
>     auto& style = layoutBox.style();
>-    auto containingBlockWidth = usedValues.containingBlockWidth;
>+    auto containingBlockWidth = usedValues.containingBlockWidth.valueOr(0);
>     auto computedHorizontalMargin = Geometry::computedHorizontalMargin(layoutBox, usedValues);
> 
>     auto usedMarginStart = [&] {
>@@ -1042,17 +1044,19 @@ Optional<Edges> FormattingContext::Geometry::computedPadding(const Box& layoutBo
>         return WTF::nullopt;
> 
>     auto& style = layoutBox.style();
>+    auto containingBlockWidth = usedValues.containingBlockWidth.valueOr(0);
>     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Padding] -> layoutBox: " << &layoutBox);
>     return Edges {
>-        { valueForLength(style.paddingLeft(), usedValues.containingBlockWidth), valueForLength(style.paddingRight(), usedValues.containingBlockWidth) },
>-        { valueForLength(style.paddingTop(), usedValues.containingBlockWidth), valueForLength(style.paddingBottom(), usedValues.containingBlockWidth) }
>+        { valueForLength(style.paddingLeft(), containingBlockWidth), valueForLength(style.paddingRight(), containingBlockWidth) },
>+        { valueForLength(style.paddingTop(), containingBlockWidth), valueForLength(style.paddingBottom(), containingBlockWidth) }
>     };
> }
> 
> ComputedHorizontalMargin FormattingContext::Geometry::computedHorizontalMargin(const Box& layoutBox, UsedHorizontalValues usedValues)
> {
>     auto& style = layoutBox.style();
>-    return { computedValueIfNotAuto(style.marginStart(), usedValues.containingBlockWidth), computedValueIfNotAuto(style.marginEnd(), usedValues.containingBlockWidth) };
>+    auto containingBlockWidth = usedValues.containingBlockWidth.valueOr(0);
>+    return { computedValueIfNotAuto(style.marginStart(), containingBlockWidth), computedValueIfNotAuto(style.marginEnd(), containingBlockWidth) };
> }
> 
> ComputedVerticalMargin FormattingContext::Geometry::computedVerticalMargin(const LayoutState& layoutState, const Box& layoutBox)
>diff --git a/Source/WebCore/layout/LayoutUnits.h b/Source/WebCore/layout/LayoutUnits.h
>index 61bae249246..c33c5527626 100644
>--- a/Source/WebCore/layout/LayoutUnits.h
>+++ b/Source/WebCore/layout/LayoutUnits.h
>@@ -125,14 +125,14 @@ struct VerticalGeometry {
> };
> 
> struct UsedHorizontalValues {
>-    explicit UsedHorizontalValues(LayoutUnit containingBlockWidth, Optional<LayoutUnit> width, Optional<UsedHorizontalMargin> margin)
>+    explicit UsedHorizontalValues(Optional<LayoutUnit> containingBlockWidth, Optional<LayoutUnit> width, Optional<UsedHorizontalMargin> margin)
>         : containingBlockWidth(containingBlockWidth)
>         , width(width)
>         , margin(margin)
>         {
>         }
> 
>-    LayoutUnit containingBlockWidth;
>+    Optional<LayoutUnit> containingBlockWidth;
>     Optional<LayoutUnit> width;
>     Optional<UsedHorizontalMargin> margin;
> };
>diff --git a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>index 3d97b03e9f0..d4359d2169f 100644
>--- a/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/blockformatting/BlockFormattingContextGeometry.cpp
>@@ -136,7 +136,7 @@ WidthAndMargin BlockFormattingContext::Geometry::inFlowNonReplacedWidthAndMargin
> 
>         auto& style = layoutBox.style();
>         auto* containingBlock = layoutBox.containingBlock();
>-        auto containingBlockWidth = usedValues.containingBlockWidth;
>+        auto containingBlockWidth = usedValues.containingBlockWidth.valueOr(0);
>         auto& displayBox = layoutState.displayBoxForLayoutBox(layoutBox);
> 
>         auto width = computedValueIfNotAuto(usedValues.width ? Length { usedValues.width.value(), Fixed } : style.logicalWidth(), containingBlockWidth);
>diff --git a/Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp b/Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp
>index e777864ec92..e899cf61580 100644
>--- a/Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/inlineformatting/InlineFormattingContextGeometry.cpp
>@@ -53,7 +53,7 @@ WidthAndMargin InlineFormattingContext::Geometry::inlineBlockWidthAndMargin(Layo
>     // If 'width' is 'auto', the used value is the shrink-to-fit width as for floating elements.
>     // A computed value of 'auto' for 'margin-left' or 'margin-right' becomes a used value of '0'.
>     // #1
>-    auto width = computedValueIfNotAuto(formattingContextRoot.style().logicalWidth(), usedValues.containingBlockWidth);
>+    auto width = computedValueIfNotAuto(formattingContextRoot.style().logicalWidth(), usedValues.containingBlockWidth.valueOr(0));
>     if (!width)
>         width = shrinkToFitWidth(layoutState, formattingContextRoot, usedValues);
>

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194428: 361493