Attachment 359699 Details for Bug 193655 – Patch

[patch] Patch

bug-193655-20190121111525.patch (text/plain), 8.96 KB, created by zalan on 2019-01-21 11:15:32 PST

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-01-21 11:15:32 PST

Size: 8.96 KB

patch

obsolete

>Subversion Revision: 240213
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index f5a5eaa8668ee5d0a4c9e67c066ecda67193a2ff..edd15b1f54ac7e406b87e70cbc2719039f83f207 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,24 @@
>+2019-01-21  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][Floats] Take float top position into account when computing containing block height.
>+        https://bugs.webkit.org/show_bug.cgi?id=193655
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When computing the containing block height, we take the first in-flow child's top position and use it as the base position.
>+        However when the first in-flow child clears a previous sibling, its vertical position is not necessarily the correct base for
>+        computing the containing block's height. Let's take the relevant floats into account as well.
>+
>+        Test: fast/block/float/float-first-child-and-clear-sibling.html
>+
>+        * layout/FormattingContextGeometry.cpp:
>+        (WebCore::Layout::contentHeightForFormattingContextRoot):
>+        * layout/floats/FloatingContext.cpp:
>+        (WebCore::Layout::FloatingContext::verticalPositionWithClearance const):
>+        * layout/floats/FloatingState.cpp:
>+        (WebCore::Layout::FloatingState::top const):
>+        * layout/floats/FloatingState.h:
>+
> 2019-01-19  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][BFC] <body>'s overflow property value is propagated to viewport
>diff --git a/Source/WebCore/layout/FormattingContextGeometry.cpp b/Source/WebCore/layout/FormattingContextGeometry.cpp
>index 93396f0d35a51ccf0c4114553222d12c3579f83f..e8479e78e82d605bc0958f6d03880bfec7bef0a9 100644
>--- a/Source/WebCore/layout/FormattingContextGeometry.cpp
>+++ b/Source/WebCore/layout/FormattingContextGeometry.cpp
>@@ -129,9 +129,14 @@ static LayoutUnit contentHeightForFormattingContextRoot(const LayoutState& layou
>         formattingContextRoot = &layoutBox.formattingContextRoot();
>     }
> 
>-    auto floatsBottom = layoutState.establishedFormattingState(*formattingContextRoot).floatingState().bottom(*formattingContextRoot);
>-    if (floatsBottom)
>-        bottom = std::max<LayoutUnit>(*floatsBottom, bottom);
>+    auto& floatingState = layoutState.establishedFormattingState(*formattingContextRoot).floatingState();
>+    auto floatBottom = floatingState.bottom(*formattingContextRoot);
>+    if (floatBottom) {
>+        bottom = std::max<LayoutUnit>(*floatBottom, bottom);
>+        auto floatTop = floatingState.top(*formattingContextRoot);
>+        ASSERT(floatTop);
>+        top = std::min<LayoutUnit>(*floatTop, top);
>+    }
> 
>     auto computedHeight = bottom - top;
>     LOG_WITH_STREAM(FormattingContextLayout, stream << "[Height] -> content height for formatting context root -> height(" << computedHeight << "px) layoutBox("<< &layoutBox << ")");
>diff --git a/Source/WebCore/layout/floats/FloatingContext.cpp b/Source/WebCore/layout/floats/FloatingContext.cpp
>index bef4e352743dc83da48ed9f954e2571f1c44bdb2..086a5d1d94f08871fd43c2493b6b0cd2df318975 100644
>--- a/Source/WebCore/layout/floats/FloatingContext.cpp
>+++ b/Source/WebCore/layout/floats/FloatingContext.cpp
>@@ -249,7 +249,10 @@ Optional<Position> FloatingContext::verticalPositionWithClearance(const Box& lay
>         rootRelativeTop += clearance;
>         ASSERT(*floatBottom == rootRelativeTop);
> 
>-        // The return vertical position is in the containing block's coordinate system.
>+        // The return vertical position is in the containing block's coordinate system. Convert it to the formatting root's coordinate system if needed.
>+        if (layoutBox.containingBlock() == &m_floatingState.root())
>+            return Position { rootRelativeTop };
>+
>         auto containingBlockRootRelativeTop = FormattingContext::mapTopLeftToAncestor(layoutState, *layoutBox.containingBlock(), downcast<Container>(m_floatingState.root())).y;
>         return Position { rootRelativeTop - containingBlockRootRelativeTop };
>     };
>diff --git a/Source/WebCore/layout/floats/FloatingState.cpp b/Source/WebCore/layout/floats/FloatingState.cpp
>index 2f7279e7705bd3594eba423fe195676a52939274..ab81002738781b2fbed20009acdba27f4c21383f 100644
>--- a/Source/WebCore/layout/floats/FloatingState.cpp
>+++ b/Source/WebCore/layout/floats/FloatingState.cpp
>@@ -184,6 +184,27 @@ Optional<PositionInContextRoot> FloatingState::bottom(const Box& formattingConte
>     return bottom;
> }
> 
>+Optional<PositionInContextRoot> FloatingState::top(const Box& formattingContextRoot) const
>+{
>+    if (m_floats.isEmpty())
>+        return { };
>+
>+    Optional<PositionInContextRoot> top;
>+    for (auto& floatItem : m_floats) {
>+        // Ignore floats from ancestor formatting contexts when the floating state is inherited.
>+        if (!floatItem.isDescendantOfFormattingRoot(formattingContextRoot))
>+            continue;
>+
>+        auto floatTop = floatItem.rectWithMargin().top();
>+        if (top) {
>+            top = std::max<PositionInContextRoot>(*top, { floatTop });
>+            continue;
>+        }
>+        top = PositionInContextRoot { floatTop };
>+    }
>+    return top;
>+}
>+
> }
> }
> #endif
>diff --git a/Source/WebCore/layout/floats/FloatingState.h b/Source/WebCore/layout/floats/FloatingState.h
>index 023d04f15f3da54e62c6040131643555a7953adb..0efae84d73b5ac2933fe515896bdaa42f25a88da 100644
>--- a/Source/WebCore/layout/floats/FloatingState.h
>+++ b/Source/WebCore/layout/floats/FloatingState.h
>@@ -54,6 +54,7 @@ public:
> 
>     const Box& root() const { return *m_formattingContextRoot; }
> 
>+    Optional<PositionInContextRoot> top(const Box& formattingContextRoot) const;
>     Optional<PositionInContextRoot> leftBottom(const Box& formattingContextRoot) const;
>     Optional<PositionInContextRoot> rightBottom(const Box& formattingContextRoot) const;
>     Optional<PositionInContextRoot> bottom(const Box& formattingContextRoot) const;
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 3b4aa661aedb6523d6d47b511690f06c943639b3..e9aea22090dbc5910234e08653e616d8daa8ca2c 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,12 @@
>+2019-01-21  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][Floats] Take float top position into account when computing containing block height.
>+        https://bugs.webkit.org/show_bug.cgi?id=193655
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * LayoutReloaded/misc/LFC-passing-tests.txt:
>+
> 2019-01-19  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][BFC] <body>'s overflow property value is propagated to viewport
>diff --git a/Tools/LayoutReloaded/misc/LFC-passing-tests.txt b/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>index 86130c596b69c0d688a4f54a1057115036b3bbf2..e8592bd0eb4325578c6e63cdc24cd7d3fe871c8b 100644
>--- a/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>+++ b/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>@@ -123,6 +123,7 @@ fast/block/float/crash-when-intruding-float-has-anonymous-parent-and-detach.html
> fast/block/float/float-in-descendant-formatting-context.html
> fast/block/float/floats-with-negative-horizontal-margin.html
> fast/block/float/float-forced-below-other-floats.html
>+fast/block/float/float-first-child-and-clear-sibling.html
> fast/block/margin-collapse/002.html
> fast/block/margin-collapse/003.html
> fast/block/margin-collapse/026.html
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index f014182038a730457fde1bfa764f66b26e5776a4..61a3e3102fe71d12d4c9ca609538bcc23897e4ca 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-01-21  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][Floats] Take float top position into account when computing containing block height.
>+        https://bugs.webkit.org/show_bug.cgi?id=193655
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/block/float/float-first-child-and-clear-sibling-expected.html: Added.
>+        * fast/block/float/float-first-child-and-clear-sibling.html: Added.
>+
> 2019-01-20  Michael Catanzaro  <mcatanzaro@igalia.com>
> 
>         Unreviewed, rolling out r240209.
>diff --git a/LayoutTests/fast/block/float/float-first-child-and-clear-sibling-expected.html b/LayoutTests/fast/block/float/float-first-child-and-clear-sibling-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..5064476aed07c0ab1b802bfee66a6ba3a61de1db
>--- /dev/null
>+++ b/LayoutTests/fast/block/float/float-first-child-and-clear-sibling-expected.html
>@@ -0,0 +1,2 @@
>+<div style="height: 20px; width: 50px; background-color: blue;"></div>
>+<div style="height: 20px; width: 50px; background-color: green;"></div>
>diff --git a/LayoutTests/fast/block/float/float-first-child-and-clear-sibling.html b/LayoutTests/fast/block/float/float-first-child-and-clear-sibling.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..79b3185068ed2c51d7dce70a3dc7f892c1605e72
>--- /dev/null
>+++ b/LayoutTests/fast/block/float/float-first-child-and-clear-sibling.html
>@@ -0,0 +1,4 @@
>+<div style="overflow: hidden;">
>+    <div style="height: 20px; width: 50px; background-color: blue; float: left;"></div>
>+    <div style="height: 20px; width: 50px; background-color: green; clear: left;"></div>
>+</div>
>\ No newline at end of file

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193655: 359699