Changeset 286820 in webkit

Timestamp:

Dec 9, 2021, 5:02:59 PM (5 years ago)

Author:

Alan Bujtas

Message:

[LFC][IFC] Stop using the last-bidi value for opaque inline items
​https://bugs.webkit.org/show_bug.cgi?id=234043

Reviewed by Antti Koivisto.

Now that we only need to guess the bidi level for empty inline boxes, let's stop applying the "keep tracking
the last bidi level from the end" logic. It may work for non-empty inline boxes (when the last bidi value
comes from an actual content run), but it is somewhat incorrect for empty inline boxes.

e.g. <span id=visually-second>&#8238;END OF CONTENT</span><span id=visually-first></span>

The "visually-first" inline box's "guessed" bidi level comes from the root direction (since it's the very list run)
making it LTR while the RTL override character (&#8238;) makes it RTL. It produces incorrect visual ordering for
these inline boxes.

• layout/formattingContexts/inline/InlineItemsBuilder.cpp:

(WebCore::Layout::InlineItemsBuilder::breakAndComputeBidiLevels):

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• layout/formattingContexts/inline/InlineItemsBuilder.cpp (modified) (4 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2021-12-09  Alan Bujtas  <zalan@apple.com>
+
+        [LFC][IFC] Stop using the last-bidi value for opaque inline items
+        https://bugs.webkit.org/show_bug.cgi?id=234043
+
+        Reviewed by Antti Koivisto.
+
+        Now that we only need to guess the bidi level for empty inline boxes, let's stop applying the "keep tracking
+        the last bidi level from the end" logic. It may work for non-empty inline boxes (when the last bidi value
+        comes from an actual content run), but it is somewhat incorrect for empty inline boxes.
+
+          e.g. <span id=visually-second>&#8238;END OF CONTENT</span><span id=visually-first></span>
+
+        The "visually-first" inline box's "guessed" bidi level comes from the root direction (since it's the very list run)
+        making it LTR while the RTL override character (&#8238;) makes it RTL. It produces incorrect visual ordering for
+        these inline boxes.
+
+        * layout/formattingContexts/inline/InlineItemsBuilder.cpp:
+        (WebCore::Layout::InlineItemsBuilder::breakAndComputeBidiLevels):
+
 2021-12-09  Alex Christensen  <achristensen@webkit.org>
 

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

@@ -289 +289 @@
             for (; inlineItemIndex < inlineItemOffsets.size(); ++inlineItemIndex) {
                 auto offset = inlineItemOffsets[inlineItemIndex];
+                auto& inlineItem = inlineItems[inlineItemIndex];
                 if (!offset) {
                     // This is an opaque item. Let's post-process it.
                     hasSeenOpaqueItem = true;
+                    inlineItem.setBidiLevel(bidiLevelForRange);
                     continue;
                 }
@@ -298 +300 @@
                     break;
                 }
-                auto& inlineItem = inlineItems[inlineItemIndex];
                 inlineItem.setBidiLevel(bidiLevelForRange);
                 if (!inlineItem.isText())
@@ -325 +326 @@
         Vector<InlineBoxHasContent> inlineBoxContentFlagStack;
         inlineBoxContentFlagStack.reserveInitialCapacity(inlineItems.size());
-        auto lastBidiLevel = rootBidiLevel;
         for (auto index = inlineItems.size(); index--;) {
             auto& inlineItem = inlineItems[index];
             if (inlineItemOffsets[index]) {
-                lastBidiLevel = inlineItem.bidiLevel();
                 inlineBoxContentFlagStack.fill(InlineBoxHasContent::Yes);
                 continue;
@@ -336 +335 @@
                 ASSERT(!inlineBoxContentFlagStack.isEmpty());
                 // Inline box start (e.g <span>) uses its content bidi level (next inline item).
-                inlineItems[index].setBidiLevel(inlineBoxContentFlagStack.takeLast() == InlineBoxHasContent::Yes ? InlineItem::opaqueBidiLevel : lastBidiLevel);
+                if (inlineBoxContentFlagStack.takeLast() == InlineBoxHasContent::Yes)
+                    inlineItems[index].setBidiLevel(InlineItem::opaqueBidiLevel);
                 continue;
             }