Attachment 359610 Details for Bug 193613 – Patch

[patch] Patch

bug-193613-20190119074710.patch (text/plain), 10.78 KB, created by zalan on 2019-01-19 07:47:16 PST

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2019-01-19 07:47:16 PST

Size: 10.78 KB

patch

obsolete

>Subversion Revision: 240146
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index c9b21f265182fc6dcd3652430d8dd738e5186641..abbcdf15ec9f5f57c2062223693a52ec92d0c357 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,30 @@
>+2019-01-19  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][Floats] Ensure that floats in FloatingContext::m_floats are always horizontally ordered.
>+        https://bugs.webkit.org/show_bug.cgi?id=193613
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Float items in m_floats list should stay in horizontal position order (left/right edge). 
>+
>+        When adding a new float item to floating state list, we have to ensure that it is definitely the left(right)-most item.
>+        Normally it is, but negative horizontal margins can push the float box beyond another float box.
>+
>+        <div style="float: left; height: 10px; width: 10px;"></div>
>+        <div style="float: left; height: 10px; width: 10px; margin-left: -80px;"></div>
>+
>+        The second float's right edge beyond the first float' left edge. THe second float is not the right(inner)-most float anymore. 
>+
>+        Test: fast/block/float/floats-with-negative-horizontal-margin.html
>+
>+        * layout/floats/FloatingContext.cpp:
>+        (WebCore::Layout::areFloatsHorizontallySorted):
>+        (WebCore::Layout::FloatingContext::positionForFloat const):
>+        (WebCore::Layout::FloatingContext::positionForFloatAvoiding const):
>+        (WebCore::Layout::FloatingContext::verticalPositionWithClearance const):
>+        * layout/floats/FloatingState.cpp:
>+        (WebCore::Layout::FloatingState::append):
>+
> 2019-01-18  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][BFC][MarginCollapsing] Collapsing through should not ignore floats.
>diff --git a/Source/WebCore/layout/floats/FloatingContext.cpp b/Source/WebCore/layout/floats/FloatingContext.cpp
>index 4e9f2cc2c0186ab69fac969c17ff4aa73691ae9d..4eabda431e03b3de6c70012aedcecdae2b9a20bb 100644
>--- a/Source/WebCore/layout/floats/FloatingContext.cpp
>+++ b/Source/WebCore/layout/floats/FloatingContext.cpp
>@@ -111,6 +111,29 @@ static Iterator end(const FloatingState& floatingState)
>     return Iterator(floatingState.floats(), WTF::nullopt);
> }
> 
>+#ifndef NDEBUG
>+static bool areFloatsHorizontallySorted(const FloatingState& floatingState)
>+{
>+    auto& floats = floatingState.floats();
>+    auto rightEdgeOfLeftFloats = LayoutUnit::min();
>+    auto leftEdgeOfRightFloats = LayoutUnit::max();
>+    for (auto& floatItem : floats) {
>+        if (floatItem.isLeftPositioned()) {
>+            auto rightEdge = floatItem.rectWithMargin().right();
>+            if (rightEdge < rightEdgeOfLeftFloats)
>+                return false;
>+            rightEdgeOfLeftFloats = rightEdge;
>+        } else {
>+            auto leftEdge = floatItem.rectWithMargin().left();
>+            if (leftEdge > leftEdgeOfRightFloats)
>+                return false;
>+            leftEdgeOfRightFloats = leftEdge;
>+        }
>+    }
>+    return true;
>+}
>+#endif
>+
> FloatingContext::FloatingContext(FloatingState& floatingState)
>     : m_floatingState(floatingState)
> {
>@@ -119,6 +142,7 @@ FloatingContext::FloatingContext(FloatingState& floatingState)
> Point FloatingContext::positionForFloat(const Box& layoutBox) const
> {
>     ASSERT(layoutBox.isFloatingPositioned());
>+    ASSERT(areFloatsHorizontallySorted(m_floatingState));
> 
>     if (m_floatingState.isEmpty()) {
>         auto& displayBox = layoutState().displayBoxForLayoutBox(layoutBox);
>@@ -148,6 +172,7 @@ Optional<Point> FloatingContext::positionForFloatAvoiding(const Box& layoutBox)
>     ASSERT(layoutBox.establishesBlockFormattingContext());
>     ASSERT(!layoutBox.isFloatingPositioned());
>     ASSERT(!layoutBox.hasFloatClear());
>+    ASSERT(areFloatsHorizontallySorted(m_floatingState));
> 
>     if (m_floatingState.isEmpty())
>         return { };
>@@ -161,6 +186,7 @@ Optional<Position> FloatingContext::verticalPositionWithClearance(const Box& lay
> {
>     ASSERT(layoutBox.hasFloatClear());
>     ASSERT(layoutBox.isBlockLevelBox());
>+    ASSERT(areFloatsHorizontallySorted(m_floatingState));
> 
>     if (m_floatingState.isEmpty())
>         return { };
>diff --git a/Source/WebCore/layout/floats/FloatingState.cpp b/Source/WebCore/layout/floats/FloatingState.cpp
>index 67ac5d40781cb7a95bde8580158f61f0c427f596..184fa994335b569c965216b7f485e35d48474a29 100644
>--- a/Source/WebCore/layout/floats/FloatingState.cpp
>+++ b/Source/WebCore/layout/floats/FloatingState.cpp
>@@ -82,8 +82,29 @@ void FloatingState::append(const Box& layoutBox)
>     ASSERT(is<Container>(*m_formattingContextRoot));
>     ASSERT(belongsToThisFloatingContext(layoutBox, *m_formattingContextRoot));
>     ASSERT(is<Container>(*m_formattingContextRoot));
>-
>-    m_floats.append({ layoutBox, *this });
>+    
>+    auto newFloatItem = FloatItem { layoutBox, *this };
>+    if (m_floats.isEmpty())
>+        return m_floats.append(newFloatItem);
>+
>+    auto& displayBox = m_layoutState.displayBoxForLayoutBox(layoutBox);
>+    auto isLeftPositioned = layoutBox.isLeftFloatingPositioned(); 
>+    // When adding a new float item to the list, we have to ensure that it is definitely the left(right)-most item.
>+    // Normally it is, but negative horizontal margins can push the float box beyond another float box.
>+    // Float items in m_floats list should stay in horizontal position order (left/right edge). 
>+    auto hasNegativeHorizontalMargin = (isLeftPositioned && displayBox.marginStart() < 0) || (!isLeftPositioned && displayBox.marginEnd() < 0);
>+    if (!hasNegativeHorizontalMargin)
>+        return m_floats.append(newFloatItem); 
>+
>+    for (int i = m_floats.size() - 1; i >= 0; --i) {
>+        auto& floatItem = m_floats[i];
>+        if (isLeftPositioned^floatItem.isLeftPositioned())
>+            continue;
>+        if ((isLeftPositioned && newFloatItem.rectWithMargin().right() >= floatItem.rectWithMargin().right())
>+            || (!isLeftPositioned && newFloatItem.rectWithMargin().left() <= floatItem.rectWithMargin().left()))
>+            return m_floats.insert(i + 1, newFloatItem);
>+    }
>+    return m_floats.insert(0, newFloatItem);
> }
> 
> FloatingState::Constraints FloatingState::constraints(PositionInContextRoot verticalPosition, const Box& formattingContextRoot) const
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index 51421ce988098771c149b34ee22611ce84c011cd..955a78fa94be965b5065c323a3caccaa787daeab 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,12 @@
>+2019-01-19  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][Floats] Ensure that floats in FloatingContext::m_floats are always horizontally ordered.
>+        https://bugs.webkit.org/show_bug.cgi?id=193613
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * LayoutReloaded/misc/LFC-passing-tests.txt:
>+
> 2019-01-18  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][BFC][MarginCollapsing] Collapsing through should not ignore floats.
>diff --git a/Tools/LayoutReloaded/misc/LFC-passing-tests.txt b/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>index 5cfe2d18318af2d4c78c201aa13e1e3f3d5231ae..130716612deda70697b5e0edf4622e6397a45454 100644
>--- a/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>+++ b/Tools/LayoutReloaded/misc/LFC-passing-tests.txt
>@@ -121,6 +121,7 @@ fast/block/float/float-with-anonymous-previous-sibling.html
> fast/block/float/floats-not-cleared-crash.html
> 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/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 cbcbd31efcc44c612a8ff24418aa4dfd21138cad..620d95e39881d0dbf771355b5d9deeda598cb147 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,13 @@
>+2019-01-19  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][Floats] Ensure that floats in FloatingContext::m_floats are always horizontally ordered.
>+        https://bugs.webkit.org/show_bug.cgi?id=193613
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * fast/block/float/floats-with-negative-horizontal-margin-expected.html: Added.
>+        * fast/block/float/floats-with-negative-horizontal-margin.html: Added.
>+
> 2019-01-18  Zalan Bujtas  <zalan@apple.com>
> 
>         [LFC][BFC][MarginCollapsing] Collapsing through should not ignore floats.
>diff --git a/LayoutTests/fast/block/float/floats-with-negative-horizontal-margin-expected.html b/LayoutTests/fast/block/float/floats-with-negative-horizontal-margin-expected.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..c7a5a1a1a9f98b8c94eb8f2d49843e0a7680f337
>--- /dev/null
>+++ b/LayoutTests/fast/block/float/floats-with-negative-horizontal-margin-expected.html
>@@ -0,0 +1,11 @@
>+<div style="position: relative; width: 500px">
>+  <div style="position: absolute; background-color: red; height: 100px; width: 100px;"></div>
>+  <div style="position: absolute; left: 20px; background-color: green; height: 150px; width: 60px;"></div>
>+  <div style="position: absolute; left: 0px; background-color: blue; height: 50px; width: 40px;"></div>
>+  <div style="position: absolute; left: 100px; background-color: brown; height: 30px; width: 30px;"></div>
>+
>+  <div style="position: absolute; right: 0px; background-color: red; height: 100px; width: 100px;"></div>
>+  <div style="position: absolute; right: 20px; background-color: green; height: 150px; width: 60px;"></div>
>+  <div style="position: absolute; right: 0px; background-color: blue; height: 50px; width: 40px;"></div>
>+  <div style="position: absolute; right: 100px; background-color: brown; height: 30px; width: 30px;"></div>
>+</div>
>\ No newline at end of file
>diff --git a/LayoutTests/fast/block/float/floats-with-negative-horizontal-margin.html b/LayoutTests/fast/block/float/floats-with-negative-horizontal-margin.html
>new file mode 100644
>index 0000000000000000000000000000000000000000..a9fe298a74f00a7893f8402fddcf47e56cb595d9
>--- /dev/null
>+++ b/LayoutTests/fast/block/float/floats-with-negative-horizontal-margin.html
>@@ -0,0 +1,10 @@
>+<div style="width: 500px;">
>+  <div style="float: left; background-color: red; height: 100px; width: 100px;"></div>
>+  <div style="float: right; background-color: red; height: 100px; width: 100px;"></div>
>+  <div style="float: left; background-color: green; height: 150px; width: 60px; margin-left: -80px;"></div>
>+  <div style="float: left; background-color: blue; height: 50px; width: 40px; margin-left: -100px;"></div>
>+  <div style="float: left; background-color: brown; height: 30px; width: 30px;"></div>
>+  <div style="float: right; background-color: green; height: 150px; width: 60px; margin-right: -80px"></div>
>+  <div style="float: right; background-color: blue; height: 50px; width: 40px; margin-right: -100px"></div>
>+  <div style="float: right; background-color: brown; height: 30px; width: 30px;"></div>
>+</div>
>\ No newline at end of file

Flags:

koivisto: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 193613: 359610