[LFC][Floats] Ensure that floats in FloatingContext::m_floats are always horizontally...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 20 Jan 2019 05:26:40 +0000 (05:26 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 20 Jan 2019 05:26:40 +0000 (05:26 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193613

Reviewed by Antti Koivisto.

Source/WebCore:

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):

Tools:

* LayoutReloaded/misc/LFC-passing-tests.txt:

LayoutTests:

* fast/block/float/floats-with-negative-horizontal-margin-expected.html: Added.
* fast/block/float/floats-with-negative-horizontal-margin.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@240207 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/fast/block/float/floats-with-negative-horizontal-margin-expected.html [new file with mode: 0644]
LayoutTests/fast/block/float/floats-with-negative-horizontal-margin.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/layout/floats/FloatingContext.cpp
Source/WebCore/layout/floats/FloatingState.cpp
Tools/ChangeLog
Tools/LayoutReloaded/misc/LFC-passing-tests.txt

index f8f5759..2026a26 100644 (file)
@@ -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 Antti Koivisto.
+
+        * fast/block/float/floats-with-negative-horizontal-margin-expected.html: Added.
+        * fast/block/float/floats-with-negative-horizontal-margin.html: Added.
+
 2019-01-19  Eric Liang  <ericliang@apple.com>
 
         AXSelected attribute on RadioButton should not be settable.
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 (file)
index 0000000..c7a5a1a
--- /dev/null
@@ -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 (file)
index 0000000..a9fe298
--- /dev/null
@@ -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
index cad0844..69c2485 100644 (file)
@@ -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 Antti Koivisto.
+
+        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-19  Youenn Fablet  <youenn@apple.com>
 
         getUserMedia with a deviceId exact constraint with an empty string value should succeed
index 4e9f2cc..bef4e35 100644 (file)
@@ -111,6 +111,38 @@ 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();
+    WTF::Optional<LayoutUnit> leftBottom;
+    WTF::Optional<LayoutUnit> rightBottom;
+
+    for (auto& floatItem : floats) {
+        if (floatItem.isLeftPositioned()) {
+            auto rightEdge = floatItem.rectWithMargin().right();
+            if (rightEdge < rightEdgeOfLeftFloats) {
+                if (leftBottom && floatItem.rectWithMargin().top() < *leftBottom)
+                    return false;
+            }
+            leftBottom = floatItem.rectWithMargin().bottom();
+            rightEdgeOfLeftFloats = rightEdge;
+        } else {
+            auto leftEdge = floatItem.rectWithMargin().left();
+            if (leftEdge > leftEdgeOfRightFloats) {
+                if (rightBottom && floatItem.rectWithMargin().top() < *rightBottom)
+                    return false;
+            }
+            rightBottom = floatItem.rectWithMargin().bottom();
+            leftEdgeOfRightFloats = leftEdge;
+        }
+    }
+    return true;
+}
+#endif
+
 FloatingContext::FloatingContext(FloatingState& floatingState)
     : m_floatingState(floatingState)
 {
@@ -119,6 +151,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 +181,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 +195,7 @@ Optional<Position> FloatingContext::verticalPositionWithClearance(const Box& lay
 {
     ASSERT(layoutBox.hasFloatClear());
     ASSERT(layoutBox.isBlockLevelBox());
+    ASSERT(areFloatsHorizontallySorted(m_floatingState));
 
     if (m_floatingState.isEmpty())
         return { };
index 67ac5d4..2f7279e 100644 (file)
@@ -83,7 +83,30 @@ void FloatingState::append(const Box& layoutBox)
     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) on the same vertical position.
+    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 (newFloatItem.rectWithMargin().top() < floatItem.rectWithMargin().bottom())
+            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
index ff4152e..0990b88 100644 (file)
@@ -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 Antti Koivisto.
+
+        * LayoutReloaded/misc/LFC-passing-tests.txt:
+
 2019-01-19  Antoine Quint  <graouts@apple.com>
 
         Add a POINTER_EVENTS feature flag
index 5cfe2d1..1307166 100644 (file)
@@ -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