<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.1//EN"
"http://www.w3.org/TR/xhtml11/DTD/xhtml11.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head><meta http-equiv="content-type" content="text/html; charset=utf-8" />
<title>[287130] trunk/Source/WebCore</title>
</head>
<body>

<style type="text/css"><!--
#msg dl.meta { border: 1px #006 solid; background: #369; padding: 6px; color: #fff; }
#msg dl.meta dt { float: left; width: 6em; font-weight: bold; }
#msg dt:after { content:':';}
#msg dl, #msg dt, #msg ul, #msg li, #header, #footer, #logmsg { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt;  }
#msg dl a { font-weight: bold}
#msg dl a:link    { color:#fc3; }
#msg dl a:active  { color:#ff0; }
#msg dl a:visited { color:#cc6; }
h3 { font-family: verdana,arial,helvetica,sans-serif; font-size: 10pt; font-weight: bold; }
#msg pre { overflow: auto; background: #ffc; border: 1px #fa0 solid; padding: 6px; }
#logmsg { background: #ffc; border: 1px #fa0 solid; padding: 1em 1em 0 1em; }
#logmsg p, #logmsg pre, #logmsg blockquote { margin: 0 0 1em 0; }
#logmsg p, #logmsg li, #logmsg dt, #logmsg dd { line-height: 14pt; }
#logmsg h1, #logmsg h2, #logmsg h3, #logmsg h4, #logmsg h5, #logmsg h6 { margin: .5em 0; }
#logmsg h1:first-child, #logmsg h2:first-child, #logmsg h3:first-child, #logmsg h4:first-child, #logmsg h5:first-child, #logmsg h6:first-child { margin-top: 0; }
#logmsg ul, #logmsg ol { padding: 0; list-style-position: inside; margin: 0 0 0 1em; }
#logmsg ul { text-indent: -1em; padding-left: 1em; }#logmsg ol { text-indent: -1.5em; padding-left: 1.5em; }
#logmsg > ul, #logmsg > ol { margin: 0 0 1em 0; }
#logmsg pre { background: #eee; padding: 1em; }
#logmsg blockquote { border: 1px solid #fa0; border-left-width: 10px; padding: 1em 1em 0 1em; background: white;}
#logmsg dl { margin: 0; }
#logmsg dt { font-weight: bold; }
#logmsg dd { margin: 0; padding: 0 0 0.5em 0; }
#logmsg dd:before { content:'\00bb';}
#logmsg table { border-spacing: 0px; border-collapse: collapse; border-top: 4px solid #fa0; border-bottom: 1px solid #fa0; background: #fff; }
#logmsg table th { text-align: left; font-weight: normal; padding: 0.2em 0.5em; border-top: 1px dotted #fa0; }
#logmsg table td { text-align: right; border-top: 1px dotted #fa0; padding: 0.2em 0.5em; }
#logmsg table thead th { text-align: center; border-bottom: 1px solid #fa0; }
#logmsg table th.Corner { text-align: left; }
#logmsg hr { border: none 0; border-top: 2px dashed #fa0; height: 1px; }
#header, #footer { color: #fff; background: #636; border: 1px #300 solid; padding: 6px; }
#patch { width: 100%; }
#patch h4 {font-family: verdana,arial,helvetica,sans-serif;font-size:10pt;padding:8px;background:#369;color:#fff;margin:0;}
#patch .propset h4, #patch .binary h4 {margin:0;}
#patch pre {padding:0;line-height:1.2em;margin:0;}
#patch .diff {width:100%;background:#eee;padding: 0 0 10px 0;overflow:auto;}
#patch .propset .diff, #patch .binary .diff  {padding:10px 0;}
#patch span {display:block;padding:0 10px;}
#patch .modfile, #patch .addfile, #patch .delfile, #patch .propset, #patch .binary, #patch .copfile {border:1px solid #ccc;margin:10px 0;}
#patch ins {background:#dfd;text-decoration:none;display:block;padding:0 10px;}
#patch del {background:#fdd;text-decoration:none;display:block;padding:0 10px;}
#patch .lines, .info {color:#888;background:#fff;}
--></style>
<div id="msg">
<dl class="meta">
<dt>Revision</dt> <dd><a href="http://trac.webkit.org/projects/webkit/changeset/287130">287130</a></dd>
<dt>Author</dt> <dd>zalan@apple.com</dd>
<dt>Date</dt> <dd>2021-12-16 06:40:26 -0800 (Thu, 16 Dec 2021)</dd>
</dl>

<h3>Log Message</h3>
<pre>[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):</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorelayoutformattingContextsinlineInlineDisplayContentBuildercpp">trunk/Source/WebCore/layout/formattingContexts/inline/InlineDisplayContentBuilder.cpp</a></li>
<li><a href="#trunkSourceWebCorelayoutformattingContextsinlineInlineItemsBuildercpp">trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (287129 => 287130)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -1,3 +1,19 @@
</span><ins>+2021-12-16  Alan Bujtas  <zalan@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):
+
</ins><span class="cx"> 2021-12-15  Antoine Quint  <graouts@webkit.org>
</span><span class="cx"> 
</span><span class="cx">         ActiveDOMObject::suspendIfNeeded() should not be called within constructors
</span></span></pre></div>
<a id="trunkSourceWebCorelayoutformattingContextsinlineInlineDisplayContentBuildercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineDisplayContentBuilder.cpp (287129 => 287130)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -572,6 +572,10 @@
</span><span class="cx">                 continue;
</span><span class="cx">             }
</span><span class="cx">             if (lineRun.isInlineBoxStart() || lineRun.isLineSpanningInlineBoxStart()) {
</span><ins>+                // 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.
</ins><span class="cx">                 appendInlineDisplayBoxAtBidiBoundary(layoutBox, boxes);
</span><span class="cx">                 createdDisplayBoxNodeForContainerBoxAndPushToAncestorStack(downcast<ContainerBox>(layoutBox), boxes.size() - 1, parentDisplayBoxNodeIndex, displayBoxTree, ancestorStack);
</span><span class="cx">                 continue;
</span></span></pre></div>
<a id="trunkSourceWebCorelayoutformattingContextsinlineInlineItemsBuildercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/layout/formattingContexts/inline/InlineItemsBuilder.cpp (287129 => 287130)</h4>
<pre class="diff"><span>
<span class="info">--- 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)
</span><span class="lines">@@ -334,19 +334,15 @@
</span><span class="cx">     auto setBidiLevelForOpaqueInlineItems = [&] {
</span><span class="cx">         if (!hasSeenOpaqueItem)
</span><span class="cx">             return;
</span><del>-        // Opaque items (inline items with no paragraph content) get their bidi level values from their adjacent items.
</del><ins>+        // 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).
</ins><span class="cx">         enum class InlineBoxHasContent : bool { No, Yes };
</span><span class="cx">         Vector<InlineBoxHasContent> inlineBoxContentFlagStack;
</span><span class="cx">         inlineBoxContentFlagStack.reserveInitialCapacity(inlineItems.size());
</span><span class="cx">         for (auto index = inlineItems.size(); index--;) {
</span><span class="cx">             auto& inlineItem = inlineItems[index];
</span><del>-            if (inlineItemOffsets[index]) {
-                inlineBoxContentFlagStack.fill(InlineBoxHasContent::Yes);
-                continue;
-            }
</del><span class="cx">             if (inlineItem.isInlineBoxStart()) {
</span><span class="cx">                 ASSERT(!inlineBoxContentFlagStack.isEmpty());
</span><del>-                // Inline box start (e.g <span>) uses its content bidi level (next inline item).
</del><span class="cx">                 if (inlineBoxContentFlagStack.takeLast() == InlineBoxHasContent::Yes)
</span><span class="cx">                     inlineItems[index].setBidiLevel(InlineItem::opaqueBidiLevel);
</span><span class="cx">                 continue;
</span><span class="lines">@@ -353,7 +349,6 @@
</span><span class="cx">             }
</span><span class="cx">             if (inlineItem.isInlineBoxEnd()) {
</span><span class="cx">                 inlineBoxContentFlagStack.append(InlineBoxHasContent::No);
</span><del>-                // Let's not confuse ubidi with non-content entries. Opaque runs are excluded from the visual list.
</del><span class="cx">                 inlineItem.setBidiLevel(InlineItem::opaqueBidiLevel);
</span><span class="cx">                 continue;
</span><span class="cx">             }
</span><span class="lines">@@ -361,7 +356,8 @@
</span><span class="cx">                 inlineItem.setBidiLevel(InlineItem::opaqueBidiLevel);
</span><span class="cx">                 continue;
</span><span class="cx">             }
</span><del>-            ASSERT_NOT_REACHED();
</del><ins>+            // Mark the inline box stack with "content yes", when we come across a content type of inline item.
+            inlineBoxContentFlagStack.fill(InlineBoxHasContent::Yes);
</ins><span class="cx">         }
</span><span class="cx">     };
</span><span class="cx">     setBidiLevelForOpaqueInlineItems();
</span></span></pre>
</div>
</div>

</body>
</html>