Title: [263550] trunk
Revision
263550
Author
[email protected]
Date
2020-06-25 21:17:47 -0700 (Thu, 25 Jun 2020)

Log Message

[Inline] Overlapping content when margin-right is present
https://bugs.webkit.org/show_bug.cgi?id=213629
<rdar://problem/64391403>

Reviewed by Simon Fraser.

Source/WebCore:

1. computeInlineDirectionPositionsForSegment loops through the Bidi runs and computes their logical widths.
2. Text measuring needs the current logical left position in order to compute the run width properly (e.g tab size)
3. The current logical left includes margins, boders and paddings (e.g <span style="margin: 100px;">text content</span> 'text content' starts at 100px -ignore the line offset for now)
4. The BiDi loop jumps over empty inline containers (e.g. text<span style="border: 10px solid green"></span>content) and it lacks the information to be able to resolve nested inline containers.

This patch pre-computes the spacing offset for each InlineTextBox so that we could use it later to compute the logical left position for the text measuring.

<span style="margin-right: 1px">[1]<span style="margin: 2px">[2]</span>[3]</span>[4]
[1] -> 0px offset
[2] -> 2px offset
[3] -> 4px offset
[4] -> 5px offset

Test: fast/inline/incorrect-tab-position.html

* rendering/ComplexLineLayout.cpp:
(WebCore::ComplexLineLayout::computeInlineDirectionPositionsForSegment):

LayoutTests:

* fast/inline/incorrect-tab-position-expected.html: Added.
* fast/inline/incorrect-tab-position.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (263549 => 263550)


--- trunk/LayoutTests/ChangeLog	2020-06-26 02:47:16 UTC (rev 263549)
+++ trunk/LayoutTests/ChangeLog	2020-06-26 04:17:47 UTC (rev 263550)
@@ -1,3 +1,14 @@
+2020-06-25  Zalan Bujtas  <[email protected]>
+
+        [Inline] Overlapping content when margin-right is present
+        https://bugs.webkit.org/show_bug.cgi?id=213629
+        <rdar://problem/64391403>
+
+        Reviewed by Simon Fraser.
+
+        * fast/inline/incorrect-tab-position-expected.html: Added.
+        * fast/inline/incorrect-tab-position.html: Added.
+
 2020-06-25  Karl Rackler  <[email protected]>
 
         Remove expectation for http/tests/cookies/third-party-cookie-relaxing.html as they are passing. 

Added: trunk/LayoutTests/fast/inline/incorrect-tab-position-expected.html (0 => 263550)


--- trunk/LayoutTests/fast/inline/incorrect-tab-position-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/inline/incorrect-tab-position-expected.html	2020-06-26 04:17:47 UTC (rev 263550)
@@ -0,0 +1,6 @@
+<style>
+.outer {
+  white-space: pre-wrap;
+}
+</style>
+<span class=outer>	should<span>not overlap</span></span>

Added: trunk/LayoutTests/fast/inline/incorrect-tab-position.html (0 => 263550)


--- trunk/LayoutTests/fast/inline/incorrect-tab-position.html	                        (rev 0)
+++ trunk/LayoutTests/fast/inline/incorrect-tab-position.html	2020-06-26 04:17:47 UTC (rev 263550)
@@ -0,0 +1,7 @@
+<style>
+.outer {
+  margin-right: 20px;
+  white-space: pre-wrap;
+}
+</style>
+<span class=outer>	should<span>not overlap</span></span>

Modified: trunk/Source/WebCore/ChangeLog (263549 => 263550)


--- trunk/Source/WebCore/ChangeLog	2020-06-26 02:47:16 UTC (rev 263549)
+++ trunk/Source/WebCore/ChangeLog	2020-06-26 04:17:47 UTC (rev 263550)
@@ -1,3 +1,29 @@
+2020-06-25  Zalan Bujtas  <[email protected]>
+
+        [Inline] Overlapping content when margin-right is present
+        https://bugs.webkit.org/show_bug.cgi?id=213629
+        <rdar://problem/64391403>
+
+        Reviewed by Simon Fraser.
+
+        1. computeInlineDirectionPositionsForSegment loops through the Bidi runs and computes their logical widths.
+        2. Text measuring needs the current logical left position in order to compute the run width properly (e.g tab size)
+        3. The current logical left includes margins, boders and paddings (e.g <span style="margin: 100px;">text content</span> 'text content' starts at 100px -ignore the line offset for now)
+        4. The BiDi loop jumps over empty inline containers (e.g. text<span style="border: 10px solid green"></span>content) and it lacks the information to be able to resolve nested inline containers.
+
+        This patch pre-computes the spacing offset for each InlineTextBox so that we could use it later to compute the logical left position for the text measuring.
+
+        <span style="margin-right: 1px">[1]<span style="margin: 2px">[2]</span>[3]</span>[4]
+        [1] -> 0px offset
+        [2] -> 2px offset
+        [3] -> 4px offset
+        [4] -> 5px offset
+
+        Test: fast/inline/incorrect-tab-position.html
+
+        * rendering/ComplexLineLayout.cpp:
+        (WebCore::ComplexLineLayout::computeInlineDirectionPositionsForSegment):
+
 2020-06-25  Alex Christensen  <[email protected]>
 
         REGRESSION(r256166, r260596) WKNavigationAction.request.allHTTPHeaderFields needs to contain User-Agent and Accept

Modified: trunk/Source/WebCore/rendering/ComplexLineLayout.cpp (263549 => 263550)


--- trunk/Source/WebCore/rendering/ComplexLineLayout.cpp	2020-06-26 02:47:16 UTC (rev 263549)
+++ trunk/Source/WebCore/rendering/ComplexLineLayout.cpp	2020-06-26 04:17:47 UTC (rev 263550)
@@ -852,8 +852,8 @@
     }
     return true;
 }
-    
-BidiRun* ComplexLineLayout::computeInlineDirectionPositionsForSegment(RootInlineBox* lineBox, const LineInfo& lineInfo, TextAlignMode textAlign, float& logicalLeft,
+
+BidiRun* ComplexLineLayout::computeInlineDirectionPositionsForSegment(RootInlineBox* lineBox, const LineInfo& lineInfo, TextAlignMode textAlign, float& lineLogicalLeft,
     float& availableLogicalWidth, BidiRun* firstRun, BidiRun* trailingSpaceRun, GlyphOverflowAndFallbackFontsMap& textBoxDataMap, VerticalPositionCache& verticalPositionCache,
     WordMeasurements& wordMeasurements)
 {
@@ -861,11 +861,49 @@
     bool canHangPunctuationAtStart = style().hangingPunctuation().contains(HangingPunctuation::First);
     bool canHangPunctuationAtEnd = style().hangingPunctuation().contains(HangingPunctuation::Last);
     bool isLTR = style().isLeftToRightDirection();
-    float totalLogicalWidth = lineBox->getFlowSpacingLogicalWidth();
+    float contentWidth = 0;
     unsigned expansionOpportunityCount = 0;
     bool isAfterExpansion = is<RenderRubyBase>(m_flow) ? downcast<RenderRubyBase>(m_flow).isAfterExpansion() : true;
     Vector<unsigned, 16> expansionOpportunities;
 
+    HashMap<InlineTextBox*, LayoutUnit> logicalSpacingForInlineTextBoxes;
+    auto collectSpacingLogicalWidths = [&] () {
+        auto totalSpacingWidth = LayoutUnit { };
+        // Collect the spacing positions (margin, border padding) for the textboxes by traversing the inline tree of the current line.
+        Vector<InlineBox*> queue;
+        queue.append(lineBox);
+        // 1. Visit each inline box in a preorder fashion
+        // 2. Accumulate the spacing when we find an InlineFlowBox (inline container e.g. span)
+        // 3. Add the InlineTextBoxes to the hashmap
+        while (!queue.isEmpty()) {
+            while (true) {
+                auto* inlineBox = queue.last();
+                if (is<InlineFlowBox>(inlineBox)) {
+                    auto& inlineFlowBox = downcast<InlineFlowBox>(*inlineBox);
+                    totalSpacingWidth += inlineFlowBox.marginBorderPaddingLogicalLeft();
+                    if (auto* child = inlineFlowBox.firstChild()) {
+                        queue.append(child);
+                        continue;
+                    }
+                    break;
+                }
+                if (is<InlineTextBox>(inlineBox))
+                    logicalSpacingForInlineTextBoxes.add(downcast<InlineTextBox>(inlineBox), totalSpacingWidth);
+                break;
+            }
+            while (!queue.isEmpty()) {
+                auto& inlineBox = *queue.takeLast();
+                if (is<InlineFlowBox>(inlineBox))
+                    totalSpacingWidth += downcast<InlineFlowBox>(inlineBox).marginBorderPaddingLogicalRight();
+                if (auto* nextSibling = inlineBox.nextOnLine()) {
+                    queue.append(nextSibling);
+                    break;
+                }
+            }
+        }
+    };
+    collectSpacingLogicalWidths();
+
     BidiRun* run = firstRun;
     BidiRun* previousRun = nullptr;
     for (; run; run = run->next()) {
@@ -897,7 +935,7 @@
                 float hangStartWidth = renderText.hangablePunctuationStartWidth(run->m_start);
                 availableLogicalWidth += hangStartWidth;
                 if (style().isLeftToRightDirection())
-                    logicalLeft -= hangStartWidth;
+                    lineLogicalLeft -= hangStartWidth;
                 canHangPunctuationAtStart = false;
             }
             
@@ -906,7 +944,7 @@
                 float hangEndWidth = renderText.hangablePunctuationEndWidth(run->m_stop - 1);
                 availableLogicalWidth += hangEndWidth;
                 if (!style().isLeftToRightDirection())
-                    logicalLeft -= hangEndWidth;
+                    lineLogicalLeft -= hangEndWidth;
                 canHangPunctuationAtEnd = false;
             }
             
@@ -915,13 +953,13 @@
 
             if (unsigned length = renderText.text().length()) {
                 if (!run->m_start && needsWordSpacing && isSpaceOrNewline(renderText.characterAt(run->m_start)))
-                    totalLogicalWidth += lineStyle(*renderText.parent(), lineInfo).fontCascade().wordSpacing();
+                    contentWidth += lineStyle(*renderText.parent(), lineInfo).fontCascade().wordSpacing();
                 // run->m_start == run->m_stop should only be true iff the run is a replaced run for bidi: isolate.
                 ASSERT(run->m_stop > 0 || run->m_start == run->m_stop);
                 needsWordSpacing = run->m_stop == length && !isSpaceOrNewline(renderText.characterAt(run->m_stop - 1));
             }
-
-            setLogicalWidthForTextRun(lineBox, run, renderText, totalLogicalWidth, lineInfo, textBoxDataMap, verticalPositionCache, wordMeasurements);
+            auto currentLogicalLeftPosition = logicalSpacingForInlineTextBoxes.get(&textBox) + contentWidth;
+            setLogicalWidthForTextRun(lineBox, run, renderText, currentLogicalLeftPosition, lineInfo, textBoxDataMap, verticalPositionCache, wordMeasurements);
         } else {
             canHangPunctuationAtStart = false;
             bool encounteredJustifiedRuby = false;
@@ -947,11 +985,11 @@
                 if (is<RenderRubyRun>(renderBox))
                     setMarginsForRubyRun(run, downcast<RenderRubyRun>(renderBox), previousRun ? &previousRun->renderer() : nullptr, lineInfo);
                 run->box()->setLogicalWidth(m_flow.logicalWidthForChild(renderBox));
-                totalLogicalWidth += m_flow.marginStartForChild(renderBox) + m_flow.marginEndForChild(renderBox);
+                contentWidth += m_flow.marginStartForChild(renderBox) + m_flow.marginEndForChild(renderBox);
             }
         }
 
-        totalLogicalWidth += run->box()->logicalWidth();
+        contentWidth += run->box()->logicalWidth();
         previousRun = run;
     }
 
@@ -970,10 +1008,9 @@
     if (is<RenderRubyBase>(m_flow) && !expansionOpportunityCount)
         textAlign = TextAlignMode::Center;
 
-    updateLogicalWidthForAlignment(m_flow, textAlign, lineBox, trailingSpaceRun, logicalLeft, totalLogicalWidth, availableLogicalWidth, expansionOpportunityCount);
-
+    auto totalLogicalWidth = contentWidth + lineBox->getFlowSpacingLogicalWidth();
+    updateLogicalWidthForAlignment(m_flow, textAlign, lineBox, trailingSpaceRun, lineLogicalLeft, totalLogicalWidth, availableLogicalWidth, expansionOpportunityCount);
     computeExpansionForJustifiedText(firstRun, trailingSpaceRun, expansionOpportunities, expansionOpportunityCount, totalLogicalWidth, availableLogicalWidth);
-
     return run;
 }
 
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to