[LFC][IFC] Incorrect bidi visual direction after forced line break

https://bugs.webkit.org/show_bug.cgi?id=244429 Reviewed by Antti Koivisto. When we unwind/rewind the bidi stack at a forced line break for nested inline boxes with different unicode directives, we have to go all the way up to the block level. It ensures that the new (unicode)paragraph after the line break has the same set of unicode directives as the content before the forced line break. * LayoutTests/fast/text/forced-line-break-and-block-level-unicode-expected.html: Added. * LayoutTests/fast/text/forced-line-break-and-block-level-unicode.html: Added. * Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp: (WebCore::Layout::buildBidiParagraph): LineBuilder::close: Apparently this legacy quirk does not apply to bidi content. Canonical link: https://commits.webkit.org/253872@main
WebKit · Aug 28, 2022 · ec3d96dee37186790ca2dbba4b0b0d64964b5811 · ec3d96d
1 parent 77ab8bd
commit ec3d96dee37186790ca2dbba4b0b0d64964b5811
Show file tree

Hide file tree

Showing 9 changed files with 68 additions and 38 deletions.
diff --git a/LayoutTests/fast/text/forced-line-break-and-block-level-unicode-expected.html b/LayoutTests/fast/text/forced-line-break-and-block-level-unicode-expected.html
@@ -0,0 +1,7 @@
+<style>
+div {
+    direction: rtl;
+    font-family: monospace;
+}
+</style>
+<div>first line<br>second line</div>
diff --git a/LayoutTests/fast/text/forced-line-break-and-block-level-unicode.html b/LayoutTests/fast/text/forced-line-break-and-block-level-unicode.html
@@ -0,0 +1,8 @@
+<style>
+div {
+    direction: rtl;
+    unicode-bidi: bidi-override;
+    font-family: monospace;
+}
+</style>
+<div>enil tsrif<br>enil dnoces</div>
diff --git a/LayoutTests/platform/ios/fast/borders/bidi-012-expected.txt b/LayoutTests/platform/ios/fast/borders/bidi-012-expected.txt
@@ -28,25 +28,24 @@ layer at (0,0) size 800x448
               text run at (11,121) width 139: "kkk lll mmm nnn ooo"
           RenderText {#text} at (0,0) size 0x0
         RenderBlock (floating) {P} at (16,214) size 184x166 [bgcolor=#FFFFCC] [border: (3px solid #000000)]
-          RenderInline {SPAN} at (0,0) size 155x87 [color=#000080] [border: (3px solid #000080)]
-            RenderText {#text} at (30,25) size 136x19
+          RenderInline {SPAN} at (0,0) size 151x87 [color=#000080] [border: (3px solid #000080)]
+            RenderText {#text} at (30,25) size 132x19
               text run at (30,25) width 26: "aaa "
               text run at (55,25) width 29: "bbb "
               text run at (83,25) width 26: "ccc "
               text run at (108,25) width 25 RTL: "ddd"
               text run at (132,25) width 30 RTL: "\x{202E} eee "
-              text run at (161,25) width 5: " "
-            RenderBR {BR} at (165,25) size 1x19
+            RenderBR {BR} at (161,25) size 1x19
             RenderText {#text} at (11,73) size 20x19
               text run at (11,73) width 20: "fff "
           RenderText {#text} at (49,73) size 74x19
             text run at (49,73) width 29: "ggg "
             text run at (77,73) width 29: "hhh "
             text run at (105,73) width 18: "iii "
           RenderInline {SPAN} at (0,0) size 162x87 [color=#FFA500] [border: (3px solid #FFA500)]
-            RenderText {#text} at (141,73) size 19x19
-              text run at (141,73) width 19: "jjj "
-            RenderBR {BR} at (159,73) size 1x19
+            RenderText {#text} at (141,73) size 15x19
+              text run at (141,73) width 15: "jjj"
+            RenderBR {BR} at (155,73) size 1x19
             RenderText {#text} at (11,121) size 143x19
               text run at (11,121) width 28: "kkk "
               text run at (39,121) width 63: "lll \x{202C} mmm "

diff --git a/LayoutTests/platform/ios/fast/dom/HTMLElement/bdo-expected.txt b/LayoutTests/platform/ios/fast/dom/HTMLElement/bdo-expected.txt
@@ -20,24 +20,21 @@ layer at (0,0) size 800x600
         RenderInline {BDO} at (0,0) size 224x19
           RenderText {#text} at (0,20) size 224x19
             text run at (0,20) width 224 RTL: "This sentence should be backward."
-        RenderText {#text} at (223,20) size 5x19
-          text run at (223,20) width 5: " "
-        RenderBR {BR} at (227,20) size 1x19
+        RenderText {#text} at (0,0) size 0x0
+        RenderBR {BR} at (223,20) size 1x19
         RenderInline {BDO} at (0,0) size 212x19
           RenderText {#text} at (0,40) size 212x19
             text run at (0,40) width 212: "This sentence should be forward."
-        RenderText {#text} at (211,40) size 5x19
-          text run at (211,40) width 5: " "
-        RenderBR {BR} at (215,40) size 1x19
+        RenderText {#text} at (0,0) size 0x0
+        RenderBR {BR} at (211,40) size 1x19
         RenderInline {BDO} at (0,0) size 0x19
         RenderText {#text} at (0,0) size 0x0
         RenderBR {BR} at (0,60) size 0x19
         RenderInline {BDO} at (0,0) size 12x19
           RenderText {#text} at (0,80) size 12x19
             text run at (0,80) width 12 RTL: "A"
-        RenderText {#text} at (11,80) size 5x19
-          text run at (11,80) width 5: " "
-        RenderBR {BR} at (15,80) size 1x19
+        RenderText {#text} at (0,0) size 0x0
+        RenderBR {BR} at (11,80) size 1x19
         RenderInline {BDO} at (0,0) size 297x19
           RenderText {#text} at (0,100) size 27x19
             text run at (0,100) width 27: "My "

diff --git a/LayoutTests/platform/mac/editing/selection/extend-selection-bidi-expected.txt b/LayoutTests/platform/mac/editing/selection/extend-selection-bidi-expected.txt
@@ -23,11 +23,11 @@ layer at (0,0) size 800x600
             text run at (368,1) width 27: "abc "
             text run at (423,1) width 9: "1"
             text run at (447,1) width 31 RTL: "\x{5D0}\x{5D1}\x{5D2} "
-        RenderText {#text} at (394,1) size 112x18
+        RenderText {#text} at (394,1) size 108x18
           text run at (394,1) width 30 RTL: " \x{5D3}\x{5D4}\x{5D5}"
           text run at (431,1) width 17: "23"
-          text run at (477,1) width 29: " def "
-        RenderBR {BR} at (505,1) size 1x18
+          text run at (477,1) width 25: " def"
+        RenderBR {BR} at (501,1) size 1x18
         RenderBR {BR} at (0,19) size 0x18
       RenderBlock {DIV} at (0,37) size 784x57 [border: (2px solid #FF0000)]
         RenderInline {SPAN} at (0,0) size 200x28

diff --git a/LayoutTests/platform/mac/fast/borders/bidi-012-expected.txt b/LayoutTests/platform/mac/fast/borders/bidi-012-expected.txt
@@ -28,25 +28,24 @@ layer at (0,0) size 800x447
               text run at (11,122) width 139: "kkk lll mmm nnn ooo"
           RenderText {#text} at (0,0) size 0x0
         RenderBlock (floating) {P} at (16,214) size 184x166 [bgcolor=#FFFFCC] [border: (3px solid #000000)]
-          RenderInline {SPAN} at (0,0) size 155x86 [color=#000080] [border: (3px solid #000080)]
-            RenderText {#text} at (30,26) size 136x18
+          RenderInline {SPAN} at (0,0) size 151x86 [color=#000080] [border: (3px solid #000080)]
+            RenderText {#text} at (30,26) size 132x18
               text run at (30,26) width 26: "aaa "
               text run at (55,26) width 29: "bbb "
               text run at (83,26) width 26: "ccc "
               text run at (108,26) width 25 RTL: "ddd"
               text run at (132,26) width 30 RTL: "\x{202E} eee "
-              text run at (161,26) width 5: " "
-            RenderBR {BR} at (165,26) size 1x18
+            RenderBR {BR} at (161,26) size 1x18
             RenderText {#text} at (11,74) size 20x18
               text run at (11,74) width 20: "fff "
           RenderText {#text} at (49,74) size 74x18
             text run at (49,74) width 29: "ggg "
             text run at (77,74) width 29: "hhh "
             text run at (105,74) width 18: "iii "
           RenderInline {SPAN} at (0,0) size 162x86 [color=#FFA500] [border: (3px solid #FFA500)]
-            RenderText {#text} at (141,74) size 19x18
-              text run at (141,74) width 19: "jjj "
-            RenderBR {BR} at (159,74) size 1x18
+            RenderText {#text} at (141,74) size 15x18
+              text run at (141,74) width 15: "jjj"
+            RenderBR {BR} at (155,74) size 1x18
             RenderText {#text} at (11,122) size 143x18
               text run at (11,122) width 28: "kkk "
               text run at (39,122) width 63: "lll \x{202C} mmm "

diff --git a/LayoutTests/platform/mac/fast/dom/HTMLElement/bdo-expected.txt b/LayoutTests/platform/mac/fast/dom/HTMLElement/bdo-expected.txt
@@ -20,24 +20,21 @@ layer at (0,0) size 800x600
         RenderInline {BDO} at (0,0) size 224x18
           RenderText {#text} at (0,18) size 224x18
             text run at (0,18) width 224 RTL: "This sentence should be backward."
-        RenderText {#text} at (223,18) size 5x18
-          text run at (223,18) width 5: " "
-        RenderBR {BR} at (227,18) size 1x18
+        RenderText {#text} at (0,0) size 0x0
+        RenderBR {BR} at (223,18) size 1x18
         RenderInline {BDO} at (0,0) size 212x18
           RenderText {#text} at (0,36) size 212x18
             text run at (0,36) width 212: "This sentence should be forward."
-        RenderText {#text} at (211,36) size 5x18
-          text run at (211,36) width 5: " "
-        RenderBR {BR} at (215,36) size 1x18
+        RenderText {#text} at (0,0) size 0x0
+        RenderBR {BR} at (211,36) size 1x18
         RenderInline {BDO} at (0,0) size 0x18
         RenderText {#text} at (0,0) size 0x0
         RenderBR {BR} at (0,54) size 0x18
         RenderInline {BDO} at (0,0) size 12x18
           RenderText {#text} at (0,72) size 12x18
             text run at (0,72) width 12 RTL: "A"
-        RenderText {#text} at (11,72) size 5x18
-          text run at (11,72) width 5: " "
-        RenderBR {BR} at (15,72) size 1x18
+        RenderText {#text} at (0,0) size 0x0
+        RenderBR {BR} at (11,72) size 1x18
         RenderInline {BDO} at (0,0) size 297x18
           RenderText {#text} at (0,90) size 27x18
             text run at (0,90) width 27: "My "

diff --git a/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp b/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp
@@ -271,6 +271,8 @@ static inline void buildBidiParagraph(const RenderStyle& rootStyle, const Inline
             appendTextBasedContent();
             inlineItemOffsetList.uncheckedAppend({ inlineTextBoxOffset + downcast<InlineSoftLineBreakItem>(inlineItem).position() });
         } else if (inlineItem.isHardLineBreak()) {
+            // Bidi handling requires us to close all the nested bidi contexts at the end of the line triggered by forced line breaks
+            // and re-open it for the content on the next line (i.e. paragraph handling).
             auto copyOfBidiStack = bidiContextStack;
 
             size_t blockLevelBidiContextIndex = 0;
@@ -288,17 +290,36 @@ static inline void buildBidiParagraph(const RenderStyle& rootStyle, const Inline
                     --unwindingIndex;
                 }
                 blockLevelBidiContextIndex = unwindingIndex; 
+                // and unwind the block entries as well.
+                do {
+                    ASSERT(copyOfBidiStack[unwindingIndex].isBlockLevel);
+                    handleEnterExitBidiContext(paragraphContentBuilder
+                        , copyOfBidiStack[unwindingIndex].unicodeBidi
+                        , copyOfBidiStack[unwindingIndex].isLeftToRightDirection
+                        , EnterExitType::ExitingBlock
+                        , bidiContextStack
+                    );
+                } while (unwindingIndex--);
             };
             unwindBidiContextStack();
 
             inlineItemOffsetList.uncheckedAppend({ paragraphContentBuilder.length() });
             paragraphContentBuilder.append(newlineCharacter);
 
             auto rewindBidiContextStack = [&] {
-                for (size_t index = blockLevelBidiContextIndex + 1; index < copyOfBidiStack.size(); ++index) {
+                for (size_t blockLevelIndex = 0; blockLevelIndex <= blockLevelBidiContextIndex; ++blockLevelIndex) {
+                    handleEnterExitBidiContext(paragraphContentBuilder
+                        , copyOfBidiStack[blockLevelIndex].unicodeBidi
+                        , copyOfBidiStack[blockLevelIndex].isLeftToRightDirection
+                        , EnterExitType::EnteringBlock
+                        , bidiContextStack
+                    );
+                }
+
+                for (size_t inlineLevelIndex = blockLevelBidiContextIndex + 1; inlineLevelIndex < copyOfBidiStack.size(); ++inlineLevelIndex) {
                     handleEnterExitBidiContext(paragraphContentBuilder
-                        , copyOfBidiStack[index].unicodeBidi
-                        , copyOfBidiStack[index].isLeftToRightDirection
+                        , copyOfBidiStack[inlineLevelIndex].unicodeBidi
+                        , copyOfBidiStack[inlineLevelIndex].isLeftToRightDirection
                         , EnterExitType::EnteringInlineBox
                         , bidiContextStack
                     );

diff --git a/Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.cpp b/Source/WebCore/layout/formattingContexts/inline/InlineLineBuilder.cpp
@@ -553,6 +553,8 @@ LineBuilder::InlineItemRange LineBuilder::close(const InlineItemRange& needsLayo
         // FIXME: webkit.org/b/233261
         if (isInIntrinsicWidthMode || !layoutState().isInlineFormattingContextIntegration())
             return false;
+        if (m_line.contentNeedsBidiReordering())
+            return false;
         return horizontalAvailableSpace >= m_line.contentLogicalWidth();
     };
     m_line.removeTrailingTrimmableContent(shouldApplyTrailingWhiteSpaceFollowedByBRQuirk() ? Line::ShouldApplyTrailingWhiteSpaceFollowedByBRQuirk::Yes : Line::ShouldApplyTrailingWhiteSpaceFollowedByBRQuirk::No);