Attachment 346276 Details for Bug 188222 – Patch

[patch] Patch

bug-188222-20180801074041.patch (text/plain), 9.54 KB, created by zalan on 2018-08-01 07:40:42 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: zalan

Created: 2018-08-01 07:40:42 PDT

Size: 9.54 KB

patch

obsolete

>Subversion Revision: 234439
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 9ad8a32841897586bc5d5c8296fed381069e8e95..260b99c28ca9ffabec763c61a468123b4539e6be 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,22 @@
>+2018-08-01  Zalan Bujtas  <zalan@apple.com>
>+
>+        [LFC][Floating] Use margin box consistently while placing a floating.
>+        https://bugs.webkit.org/show_bug.cgi?id=188222
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        The floating box fits when its margin box fits.
>+
>+        * layout/FloatingContext.cpp:
>+        (WebCore::Layout::FloatingContext::computePosition const):
>+        (WebCore::Layout::FloatingContext::floatingPosition const):
>+        (WebCore::Layout::FloatingContext::initialVerticalPosition const):
>+        (WebCore::Layout::FloatingContext::alignWithContainingBlock const):
>+        (WebCore::Layout::FloatingContext::alignWithFloatings const):
>+        (WebCore::Layout::FloatingPair::intersects const):
>+        * layout/displaytree/DisplayBox.h:
>+        (WebCore::Display::Box::rectWithMargin const):
>+
> 2018-07-31  Wenson Hsieh  <wenson_hsieh@apple.com>
> 
>         [iOS] Spelling suggestions cannot be selected in focused form controls when zoomed in
>diff --git a/Source/WebCore/layout/FloatingContext.cpp b/Source/WebCore/layout/FloatingContext.cpp
>index 1859d92395f44cf2bef2a4a071b7802a5a891268..6bc2271cc4a5189edfaf6de96f7aa570885b47c1 100644
>--- a/Source/WebCore/layout/FloatingContext.cpp
>+++ b/Source/WebCore/layout/FloatingContext.cpp
>@@ -118,8 +118,10 @@ Position FloatingContext::computePosition(const Box& layoutBox) const
>     ASSERT(layoutBox.isFloatingPositioned());
> 
>     // 1. No floating box on the context yet -> align it with the containing block's left/right edge. 
>-    if (m_floatingState.isEmpty())
>-        return { alignWithContainingBlock(layoutBox), layoutContext().displayBoxForLayoutBox(layoutBox)->top() };
>+    if (m_floatingState.isEmpty()) {
>+        auto* displayBox = layoutContext().displayBoxForLayoutBox(layoutBox);
>+        return { alignWithContainingBlock(layoutBox) + displayBox->marginLeft(), displayBox->top() };
>+    }
> 
>     // 2. Find the top most position where the floating fits.
>     return floatingPosition(layoutBox); 
>@@ -128,7 +130,8 @@ Position FloatingContext::computePosition(const Box& layoutBox) const
> Position FloatingContext::floatingPosition(const Box& layoutBox) const
> {
>     auto initialVerticalPosition = this->initialVerticalPosition(layoutBox);
>-    auto boxSize = layoutContext().displayBoxForLayoutBox(layoutBox)->marginBox().size();
>+    auto* displayBox = layoutContext().displayBoxForLayoutBox(layoutBox);
>+    auto marginBoxSize = displayBox->marginBox().size();
> 
>     auto end = Layout::end(layoutContext(), m_floatingState);
>     auto top = initialVerticalPosition;
>@@ -141,71 +144,78 @@ Position FloatingContext::floatingPosition(const Box& layoutBox) const
>         // Move the box horizontally so that it aligns with the current floating pair.
>         auto left = alignWithFloatings(floatings, layoutBox);
>         // Check if the box fits at this vertical position.
>-        if (!floatings.intersects({ top, left, boxSize.width(), boxSize.height() }))
>-            return { left, top };
>+        if (!floatings.intersects({ top, left, marginBoxSize.width(), marginBoxSize.height() }))
>+            return { left + displayBox->marginLeft(), top + displayBox->marginTop() };
> 
>         // Move to the next floating pair.
>     }
> 
>     // Passed all the floatings and still does not fit? 
>-    return { alignWithContainingBlock(layoutBox), top };
>+    return { alignWithContainingBlock(layoutBox) + displayBox->marginLeft(), top + displayBox->marginTop() };
> }
> 
> LayoutUnit FloatingContext::initialVerticalPosition(const Box& layoutBox) const
> {
>     // Incoming floating cannot be placed higher than existing floatings.
>     // Take the static position (where the box would go if it wasn't floating) and adjust it with the last floating.
>-    auto& displayBox = *layoutContext().displayBoxForLayoutBox(layoutBox);
>+    auto marginBoxTop = layoutContext().displayBoxForLayoutBox(layoutBox)->rectWithMargin().top();
> 
>     if (auto lastFloating = m_floatingState.last())
>-        return std::max(displayBox.top(), layoutContext().displayBoxForLayoutBox(*lastFloating)->top());
>+        return std::max(marginBoxTop, layoutContext().displayBoxForLayoutBox(*lastFloating)->rectWithMargin().top());
> 
>-    return displayBox.top();
>+    return marginBoxTop;
> }
> 
> LayoutUnit FloatingContext::alignWithContainingBlock(const Box& layoutBox) const
> {
>     // If there is no floating to align with, push the box to the left/right edge of its containing block's content box.
>     // (Either there's no floatings at all or this box does not fit at any vertical positions where the floatings are.)
>-    auto& layoutContext = m_floatingState.layoutContext();
>+    auto* displayBox = layoutContext().displayBoxForLayoutBox(layoutBox);
>     auto* containingBlock = layoutBox.containingBlock();
>     ASSERT(containingBlock == &m_formattingContextRoot || containingBlock->isDescendantOf(m_formattingContextRoot));
> 
>-    auto* containgBlockDisplayBox = layoutContext.displayBoxForLayoutBox(*containingBlock);
>+    auto* containgBlockDisplayBox = layoutContext().displayBoxForLayoutBox(*containingBlock);
> 
>     if (layoutBox.isLeftFloatingPositioned())
>         return containgBlockDisplayBox->contentBoxLeft();
> 
>-    auto boxWidth = layoutContext.displayBoxForLayoutBox(layoutBox)->marginBox().width();
>-    return containgBlockDisplayBox->contentBoxRight() - boxWidth;
>+    return containgBlockDisplayBox->contentBoxRight() - displayBox->marginBox().width();
> }
> 
> LayoutUnit FloatingContext::alignWithFloatings(const FloatingPair& floatingPair, const Box& layoutBox) const
> {
>     // Compute the horizontal position for the new floating by taking both the contining block and the current left/right floatings into account.
>+    auto* displayBox = layoutContext().displayBoxForLayoutBox(layoutBox);
>     auto& containingBlock = *layoutContext().displayBoxForLayoutBox(*layoutBox.containingBlock());
>-    auto containingBlockContentLeft = containingBlock.contentBoxLeft();
>-    auto containingBlockContentRight = containingBlock.contentBoxRight();
>-    auto marginBoxWidth = layoutContext().displayBoxForLayoutBox(layoutBox)->marginBox().width();
>+    auto containingBlockContentBoxLeft = containingBlock.contentBoxLeft();
>+    auto containingBlockContentBoxRight = containingBlock.contentBoxRight();
>+    auto marginBoxWidth = displayBox->marginBox().width();
>+
>+    auto leftAlignedBoxLeft = containingBlockContentBoxLeft;
>+    auto rightAlignedBoxLeft = containingBlockContentBoxRight - displayBox->marginBox().width();
> 
>     if (floatingPair.isEmpty()) {
>         ASSERT_NOT_REACHED();
>-        return layoutBox.isLeftFloatingPositioned() ? containingBlockContentLeft : containingBlockContentRight - marginBoxWidth;
>+        return layoutBox.isLeftFloatingPositioned() ? leftAlignedBoxLeft : rightAlignedBoxLeft;
>     }
> 
>     if (layoutBox.isLeftFloatingPositioned()) {
>-        if (auto* leftDisplayBox = floatingPair.left()) 
>-            return std::min(std::max(containingBlockContentLeft, leftDisplayBox->right()), containingBlockContentRight - marginBoxWidth);
>+        if (auto* leftDisplayBox = floatingPair.left()) {
>+            auto leftFloatingBoxRight = leftDisplayBox->rectWithMargin().right();
>+            return std::min(std::max(leftAlignedBoxLeft, leftFloatingBoxRight), rightAlignedBoxLeft);
>+        }
>         
>-        return containingBlockContentLeft;
>+        return leftAlignedBoxLeft;
>     }
> 
>     ASSERT(layoutBox.isRightFloatingPositioned());
> 
>-    if (auto* rightDisplayBox = floatingPair.right())
>-        return std::max(std::min(containingBlockContentRight, rightDisplayBox->left()) - marginBoxWidth, containingBlockContentLeft);
>+    if (auto* rightDisplayBox = floatingPair.right()) {
>+        auto rightFloatingBoxLeft = rightDisplayBox->rectWithMargin().left();
>+        return std::max(std::min(rightAlignedBoxLeft, rightFloatingBoxLeft) - marginBoxWidth, leftAlignedBoxLeft);
>+    }
> 
>-    return containingBlockContentRight - marginBoxWidth;
>+    return rightAlignedBoxLeft;
> }
> 
> static const Display::Box* floatingDisplayBox(unsigned index, const FloatingState::FloatingList& floatings, const LayoutContext& layoutContext)
>@@ -241,8 +251,8 @@ bool FloatingPair::intersects(const Display::Box::Rect& rect) const
>     auto intersects = [&](const Display::Box* floating, const Display::Box::Rect& rect) {
>         if (!floating)
>             return false;
>-        // FIXME: use margin box here.
>-        return floating->rect().intersects(rect);
>+
>+        return floating->rectWithMargin().intersects(rect);
>     };
> 
>     if (!m_leftIndex && !m_rightIndex) {
>diff --git a/Source/WebCore/layout/displaytree/DisplayBox.h b/Source/WebCore/layout/displaytree/DisplayBox.h
>index 59cc771a48bcfce2120c1a4aee79c9dcc785ae91..2fe6b56489c2050b99509a04103f064e8b0ac05b 100644
>--- a/Source/WebCore/layout/displaytree/DisplayBox.h
>+++ b/Source/WebCore/layout/displaytree/DisplayBox.h
>@@ -125,6 +125,7 @@ public:
>     LayoutUnit width() const { return borderLeft() + paddingLeft() + contentBoxWidth() + paddingRight() + borderRight(); }
>     LayoutUnit height() const { return borderTop() + paddingTop() + contentBoxHeight() + paddingBottom() + borderBottom(); }
>     Rect rect() const { return { top(), left(), width(), height() }; }
>+    Rect rectWithMargin() const { return { top() - marginTop(), left() - marginLeft(), width() + marginRight(), height() + marginBottom() }; }
> 
>     LayoutUnit marginTop() const;
>     LayoutUnit marginLeft() const;

Flags:

koivisto: review+
ews-watchlist: commit-queue-

Actions: View | Formatted Diff | Diff

Attachments on bug 188222: 346276 | 346290