Changeset 287922 in webkit

Timestamp:

Jan 12, 2022, 6:37:38 AM (5 years ago)

Author:

Alan Bujtas

Message:

[LFC][IFC] Incorrect negative margin handling (both left/right) with RTL inline base direction
​https://bugs.webkit.org/show_bug.cgi?id=235095

Reviewed by Antti Koivisto.

Source/WebCore:

The simplified negative margin handling on inline boxes does not work well with RTL inline base direction.
With LTR direction, we could just treat the negative left margin value (which pulls content to the left)
as part the "logical width" (may resulting in negative width values) and let this shorter width pull
the the adjoining content.
However this setup produces incorrect box positions when the inline base direction is RTL.
In this patch, we switch over to a more correct inline box positioning where the negative margin
affects the logical left while it does not make the run shorter anymore.

Test: fast/inline/rtl-negative-margins.html

• layout/formattingContexts/inline/InlineLine.cpp:

(WebCore::Layout::Line::appendInlineBoxStart):
(WebCore::Layout::Line::appendNonReplacedInlineLevelBox):

• layout/formattingContexts/inline/InlineLineBoxBuilder.cpp:

(WebCore::Layout::LineBoxBuilder::constructAndAlignInlineLevelBoxes):

• layout/formattingContexts/inline/InlineLineBuilder.cpp:

(WebCore::Layout::LineBuilder::layoutInlineContent):

• layout/formattingContexts/inline/InlineLineBuilder.h:
• layout/formattingContexts/inline/display/InlineDisplayLineBuilder.cpp:

(WebCore::Layout::InlineDisplayLineBuilder::build const):

LayoutTests:

• fast/inline/rtl-negative-margins-expected.html: Added.
• fast/inline/rtl-negative-margins.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/inline/rtl-negative-margins-expected.html (added)
• LayoutTests/fast/inline/rtl-negative-margins.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/layout/formattingContexts/inline/InlineLine.cpp (modified) (2 diffs)
• Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.cpp (modified) (2 diffs)
• Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.cpp (modified) (1 diff)
• Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.h (modified) (1 diff)
• Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayLineBuilder.cpp (modified) (1 diff)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2022-01-12  Alan Bujtas  <zalan@apple.com>
+
+        [LFC][IFC] Incorrect negative margin handling (both left/right) with RTL inline base direction
+        https://bugs.webkit.org/show_bug.cgi?id=235095
+
+        Reviewed by Antti Koivisto.
+
+        * fast/inline/rtl-negative-margins-expected.html: Added.
+        * fast/inline/rtl-negative-margins.html: Added.
+
 2022-01-11  Said Abou-Hallawa  <said@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2022-01-12  Alan Bujtas  <zalan@apple.com>
+
+        [LFC][IFC] Incorrect negative margin handling (both left/right) with RTL inline base direction
+        https://bugs.webkit.org/show_bug.cgi?id=235095
+
+        Reviewed by Antti Koivisto.
+
+        The simplified negative margin handling on inline boxes does not work well with RTL inline base direction.
+        With LTR direction, we could just treat the negative left margin value (which pulls content to the left)
+        as part the "logical width" (may resulting in negative width values) and let this shorter width pull
+        the the adjoining content.
+        However this setup produces incorrect box positions when the inline base direction is RTL.
+        In this patch, we switch over to a more correct inline box positioning where the negative margin
+        affects the logical left while it does not make the run shorter anymore.
+
+        Test: fast/inline/rtl-negative-margins.html
+
+        * layout/formattingContexts/inline/InlineLine.cpp:
+        (WebCore::Layout::Line::appendInlineBoxStart):
+        (WebCore::Layout::Line::appendNonReplacedInlineLevelBox):
+        * layout/formattingContexts/inline/InlineLineBoxBuilder.cpp:
+        (WebCore::Layout::LineBoxBuilder::constructAndAlignInlineLevelBoxes):
+        * layout/formattingContexts/inline/InlineLineBuilder.cpp:
+        (WebCore::Layout::LineBuilder::layoutInlineContent):
+        * layout/formattingContexts/inline/InlineLineBuilder.h:
+        * layout/formattingContexts/inline/display/InlineDisplayLineBuilder.cpp:
+        (WebCore::Layout::InlineDisplayLineBuilder::build const):
+
 2022-01-12  Nikolas Zimmermann  <nzimmermann@igalia.com>
 

trunk/Source/WebCore/layout/formattingContexts/inline/InlineLine.cpp

@@ -229 +229 @@
     // Incoming logical width includes the cloned decoration end to be able to do line breaking.
     auto borderAndPaddingEndForDecorationClone = addBorderAndPaddingEndForInlineBoxDecorationClone(inlineItem);
-    m_runs.append({ inlineItem, style, logicalLeft, logicalWidth - borderAndPaddingEndForDecorationClone });
     // Do not let negative margin make the content shorter than it already is.
     m_contentLogicalWidth = std::max(m_contentLogicalWidth, logicalLeft + logicalWidth);
+
+    auto marginStart = formattingContext().geometryForBox(inlineItem.layoutBox()).marginStart();
+    if (marginStart >= 0) {
+        m_runs.append({ inlineItem, style, logicalLeft, logicalWidth - borderAndPaddingEndForDecorationClone });
+        return;
+    }
+    // Negative margin-start pulls the content to the logical left direction.
+    m_runs.append({ inlineItem, style, logicalLeft + marginStart, logicalWidth - marginStart - borderAndPaddingEndForDecorationClone });
 }
 
@@ -352 +359 @@
 {
     resetTrailingContent();
-    m_contentLogicalWidth += marginBoxLogicalWidth;
+    // Do not let negative margin make the content shorter than it already is.
+    m_contentLogicalWidth = std::max(m_contentLogicalWidth, lastRunLogicalRight() + marginBoxLogicalWidth);
     ++m_nonSpanningInlineLevelBoxCount;
     auto marginStart = formattingContext().geometryForBox(inlineItem.layoutBox()).marginStart();

trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBoxBuilder.cpp

@@ -284 +284 @@
                 marginStart = formattingContext().geometryForBox(layoutBox).marginStart();
 #endif
-            auto adjustedLogicalStart = logicalLeft + marginStart;
+            auto adjustedLogicalStart = logicalLeft + std::max(0.0f, marginStart);
             auto logicalWidth = rootInlineBox.logicalWidth() - adjustedLogicalStart;
             auto inlineBox = InlineLevelBox::createInlineBox(layoutBox, style, adjustedLogicalStart, logicalWidth, InlineLevelBox::LineSpanningInlineBox::Yes);
@@ -297 +297 @@
             // Inline box run is based on margin box. Let's convert it to border box.
             auto marginStart = formattingContext().geometryForBox(layoutBox).marginStart();
-            auto initialLogicalWidth = rootInlineBox.logicalWidth() - (run.logicalLeft() + marginStart);
+            logicalLeft += std::max(0_lu, marginStart);
+            auto initialLogicalWidth = rootInlineBox.logicalWidth() - (logicalLeft - rootInlineBox.logicalLeft());
             ASSERT(initialLogicalWidth >= 0 || lineContent.hangingContentWidth);
             initialLogicalWidth = std::max(initialLogicalWidth, 0.f);
-            auto inlineBox = InlineLevelBox::createInlineBox(layoutBox, style, logicalLeft + marginStart, initialLogicalWidth);
+            auto inlineBox = InlineLevelBox::createInlineBox(layoutBox, style, logicalLeft, initialLogicalWidth);
             inlineBox.setIsFirstBox();
             setInitialVerticalGeometryForInlineBox(inlineBox);

trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.cpp

@@ -354 +354 @@
         , m_lineLogicalRect.width()
         , m_line.contentLogicalWidth()
+        , m_line.contentLogicalRight()
         , m_line.hangingTrailingContentWidth()
         , isLastLine

trunk/Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.h

@@ -62 +62 @@
         InlineLayoutUnit lineLogicalWidth { 0 };
         InlineLayoutUnit contentLogicalWidth { 0 };
+        InlineLayoutUnit contentLogicalRight { 0 };
         InlineLayoutUnit hangingContentWidth { 0 };
         bool isLastLineWithInlineContent { true };

trunk/Source/WebCore/layout/formattingContexts/inline/display/InlineDisplayLineBuilder.cpp

@@ -90 +90 @@
     auto contentVisualLeft = isLeftToRightDirection
         ? lineBox.rootInlineBoxAlignmentOffset()
-        : rootGeometry.contentBoxWidth() - lineOffsetFromContentBox -  lineBox.rootInlineBoxAlignmentOffset() - rootInlineBox.logicalWidth() - lineContent.hangingContentWidth;
+        : rootGeometry.contentBoxWidth() - lineOffsetFromContentBox -  lineBox.rootInlineBoxAlignmentOffset() - lineContent.contentLogicalRight;
 
     auto lineBoxRect = InlineRect { lineContent.lineLogicalTopLeft.y(), lineBoxVisualLeft, lineContent.lineLogicalWidth, lineBoxLogicalHeight };