Title: [287130] trunk/Source/WebCore
Revision
287130
Author
za...@apple.com
Date
2021-12-16 06:40:26 -0800 (Thu, 16 Dec 2021)

Log Message

[LFC][IFC] Do not use inlineItemOffsets for checking if an inline item is a "content" type.
https://bugs.webkit.org/show_bug.cgi?id=234383

Reviewed by Antti Koivisto.

This patch removes some leftover logic from when we used the inlineItemOffsets to check for opaque inline items.
Now in this loop we try to figure out if the inline box has content or not by
looking at the nested inline item types.

* layout/formattingContexts/inline/InlineDisplayContentBuilder.cpp:
(WebCore::Layout::InlineDisplayContentBuilder::processBidiContent):
* layout/formattingContexts/inline/InlineItemsBuilder.cpp:
(WebCore::Layout::InlineItemsBuilder::breakAndComputeBidiLevels):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (287129 => 287130)


--- trunk/Source/WebCore/ChangeLog	2021-12-16 11:31:09 UTC (rev 287129)
+++ trunk/Source/WebCore/ChangeLog	2021-12-16 14:40:26 UTC (rev 287130)
@@ -1,3 +1,19 @@
+2021-12-16  Alan Bujtas  <za...@apple.com>
+
+        [LFC][IFC] Do not use inlineItemOffsets for checking if an inline item is a "content" type.
+        https://bugs.webkit.org/show_bug.cgi?id=234383
+
+        Reviewed by Antti Koivisto.
+
+        This patch removes some leftover logic from when we used the inlineItemOffsets to check for opaque inline items.
+        Now in this loop we try to figure out if the inline box has content or not by
+        looking at the nested inline item types.
+
+        * layout/formattingContexts/inline/InlineDisplayContentBuilder.cpp:
+        (WebCore::Layout::InlineDisplayContentBuilder::processBidiContent):
+        * layout/formattingContexts/inline/InlineItemsBuilder.cpp:
+        (WebCore::Layout::InlineItemsBuilder::breakAndComputeBidiLevels):
+
 2021-12-15  Antoine Quint  <grao...@webkit.org>
 
         ActiveDOMObject::suspendIfNeeded() should not be called within constructors

Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineDisplayContentBuilder.cpp (287129 => 287130)


--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineDisplayContentBuilder.cpp	2021-12-16 11:31:09 UTC (rev 287129)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineDisplayContentBuilder.cpp	2021-12-16 14:40:26 UTC (rev 287130)
@@ -572,6 +572,10 @@
                 continue;
             }
             if (lineRun.isInlineBoxStart() || lineRun.isLineSpanningInlineBoxStart()) {
+                // FIXME: While we should only get here with empty inline boxes, there are
+                // some cases where the inline box has some content on the paragraph level (at bidi split) but line breaking renders it empty
+                // or their content is completely collapsed.
+                // Such inline boxes should also be handled here.
                 appendInlineDisplayBoxAtBidiBoundary(layoutBox, boxes);
                 createdDisplayBoxNodeForContainerBoxAndPushToAncestorStack(downcast<ContainerBox>(layoutBox), boxes.size() - 1, parentDisplayBoxNodeIndex, displayBoxTree, ancestorStack);
                 continue;

Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp (287129 => 287130)


--- trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp	2021-12-16 11:31:09 UTC (rev 287129)
+++ trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp	2021-12-16 14:40:26 UTC (rev 287130)
@@ -334,19 +334,15 @@
     auto setBidiLevelForOpaqueInlineItems = [&] {
         if (!hasSeenOpaqueItem)
             return;
-        // Opaque items (inline items with no paragraph content) get their bidi level values from their adjacent items.
+        // Let's not confuse ubidi with non-content entries.
+        // Opaque runs are excluded from the visual list (ie. only empty inline boxes should be kept around as bidi content -to figure out their visual order).
         enum class InlineBoxHasContent : bool { No, Yes };
         Vector<InlineBoxHasContent> inlineBoxContentFlagStack;
         inlineBoxContentFlagStack.reserveInitialCapacity(inlineItems.size());
         for (auto index = inlineItems.size(); index--;) {
             auto& inlineItem = inlineItems[index];
-            if (inlineItemOffsets[index]) {
-                inlineBoxContentFlagStack.fill(InlineBoxHasContent::Yes);
-                continue;
-            }
             if (inlineItem.isInlineBoxStart()) {
                 ASSERT(!inlineBoxContentFlagStack.isEmpty());
-                // Inline box start (e.g <span>) uses its content bidi level (next inline item).
                 if (inlineBoxContentFlagStack.takeLast() == InlineBoxHasContent::Yes)
                     inlineItems[index].setBidiLevel(InlineItem::opaqueBidiLevel);
                 continue;
@@ -353,7 +349,6 @@
             }
             if (inlineItem.isInlineBoxEnd()) {
                 inlineBoxContentFlagStack.append(InlineBoxHasContent::No);
-                // Let's not confuse ubidi with non-content entries. Opaque runs are excluded from the visual list.
                 inlineItem.setBidiLevel(InlineItem::opaqueBidiLevel);
                 continue;
             }
@@ -361,7 +356,8 @@
                 inlineItem.setBidiLevel(InlineItem::opaqueBidiLevel);
                 continue;
             }
-            ASSERT_NOT_REACHED();
+            // Mark the inline box stack with "content yes", when we come across a content type of inline item.
+            inlineBoxContentFlagStack.fill(InlineBoxHasContent::Yes);
         }
     };
     setBidiLevelForOpaqueInlineItems();
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to