<!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>[243331] trunk</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/243331">243331</a></dd>
<dt>Author</dt> <dd>zalan@apple.com</dd>
<dt>Date</dt> <dd>2019-03-21 14:43:14 -0700 (Thu, 21 Mar 2019)</dd>
</dl>

<h3>Log Message</h3>
<pre>Do not insert the first-letter anonymous container until after we've constructed the first-letter renderer.
https://bugs.webkit.org/show_bug.cgi?id=195919
<rdar://problem/48573434>

Reviewed by Brent Fulgham.

Source/WebCore:

When the container is injected too early, we might end up removing it as part of the collapsing logic
while the text renderer is being removed (replaced with the first letter + remaining text).

Test: fast/css/first-letter-and-float-crash.html

* rendering/updating/RenderTreeBuilderFirstLetter.cpp:
(WebCore::RenderTreeBuilder::FirstLetter::createRenderers):

LayoutTests:

* fast/css/first-letter-and-float-crash-expected.txt: Added.
* fast/css/first-letter-and-float-crash.html: Added.
* platform/mac/TestExpectations:</pre>

<h3>Modified Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsChangeLog">trunk/LayoutTests/ChangeLog</a></li>
<li><a href="#trunkLayoutTestsTestExpectations">trunk/LayoutTests/TestExpectations</a></li>
<li><a href="#trunkSourceWebCoreChangeLog">trunk/Source/WebCore/ChangeLog</a></li>
<li><a href="#trunkSourceWebCorerenderingupdatingRenderTreeBuilderFirstLettercpp">trunk/Source/WebCore/rendering/updating/RenderTreeBuilderFirstLetter.cpp</a></li>
</ul>

<h3>Added Paths</h3>
<ul>
<li><a href="#trunkLayoutTestsfastcssfirstletterandfloatcrashexpectedtxt">trunk/LayoutTests/fast/css/first-letter-and-float-crash-expected.txt</a></li>
<li><a href="#trunkLayoutTestsfastcssfirstletterandfloatcrashhtml">trunk/LayoutTests/fast/css/first-letter-and-float-crash.html</a></li>
</ul>

</div>
<div id="patch">
<h3>Diff</h3>
<a id="trunkLayoutTestsChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/ChangeLog (243330 => 243331)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/ChangeLog      2019-03-21 21:36:26 UTC (rev 243330)
+++ trunk/LayoutTests/ChangeLog 2019-03-21 21:43:14 UTC (rev 243331)
</span><span class="lines">@@ -1,3 +1,15 @@
</span><ins>+2019-03-21  Zalan Bujtas  <zalan@apple.com>
+
+        Do not insert the first-letter anonymous container until after we've constructed the first-letter renderer.
+        https://bugs.webkit.org/show_bug.cgi?id=195919
+        <rdar://problem/48573434>
+
+        Reviewed by Brent Fulgham.
+
+        * fast/css/first-letter-and-float-crash-expected.txt: Added.
+        * fast/css/first-letter-and-float-crash.html: Added.
+        * platform/mac/TestExpectations:
+
</ins><span class="cx"> 2019-03-21  Eric Carlson  <eric.carlson@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Add UI process WebRTC runtime logging.
</span></span></pre></div>
<a id="trunkLayoutTestsTestExpectations"></a>
<div class="modfile"><h4>Modified: trunk/LayoutTests/TestExpectations (243330 => 243331)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/TestExpectations       2019-03-21 21:36:26 UTC (rev 243330)
+++ trunk/LayoutTests/TestExpectations  2019-03-21 21:43:14 UTC (rev 243331)
</span><span class="lines">@@ -3059,3 +3059,5 @@
</span><span class="cx"> imported/w3c/web-platform-tests/css/css-lists/counter-reset-inside-display-contents.html [ ImageOnlyFailure ]
</span><span class="cx"> imported/w3c/web-platform-tests/css/css-lists/list-marker-with-lineheight-and-overflow-hidden-001.html [ ImageOnlyFailure ]
</span><span class="cx"> imported/w3c/web-platform-tests/css/css-lists/list-with-image-display-changed-001.html [ ImageOnlyFailure ]
</span><ins>+
+[ Debug ] fast/css/first-letter-and-float-crash.html [ Skip ]
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssfirstletterandfloatcrashexpectedtxt"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/first-letter-and-float-crash-expected.txt (0 => 243331)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/first-letter-and-float-crash-expected.txt                             (rev 0)
+++ trunk/LayoutTests/fast/css/first-letter-and-float-crash-expected.txt        2019-03-21 21:43:14 UTC (rev 243331)
</span><span class="lines">@@ -0,0 +1 @@
</span><ins>+Pass if no crash
</ins></span></pre></div>
<a id="trunkLayoutTestsfastcssfirstletterandfloatcrashhtml"></a>
<div class="addfile"><h4>Added: trunk/LayoutTests/fast/css/first-letter-and-float-crash.html (0 => 243331)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/LayoutTests/fast/css/first-letter-and-float-crash.html                             (rev 0)
+++ trunk/LayoutTests/fast/css/first-letter-and-float-crash.html        2019-03-21 21:43:14 UTC (rev 243331)
</span><span class="lines">@@ -0,0 +1,11 @@
</span><ins>+<style>
+:matches(foobar, .inlineContainer .floatContainer)::first-letter {
+ font-size: 10px;
+}
+</style>
+
+<span class=inlineContainer><div style="float: left" class=floatContainer>Pass if no crash</div></span>
+<script>
+if (window.testRunner)
+    testRunner.dumpAsText();
+</script>
</ins><span class="cx">\ No newline at end of file
</span></span></pre></div>
<a id="trunkSourceWebCoreChangeLog"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/ChangeLog (243330 => 243331)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/ChangeLog   2019-03-21 21:36:26 UTC (rev 243330)
+++ trunk/Source/WebCore/ChangeLog      2019-03-21 21:43:14 UTC (rev 243331)
</span><span class="lines">@@ -1,3 +1,19 @@
</span><ins>+2019-03-21  Zalan Bujtas  <zalan@apple.com>
+
+        Do not insert the first-letter anonymous container until after we've constructed the first-letter renderer.
+        https://bugs.webkit.org/show_bug.cgi?id=195919
+        <rdar://problem/48573434>
+
+        Reviewed by Brent Fulgham.
+
+        When the container is injected too early, we might end up removing it as part of the collapsing logic
+        while the text renderer is being removed (replaced with the first letter + remaining text).
+
+        Test: fast/css/first-letter-and-float-crash.html
+
+        * rendering/updating/RenderTreeBuilderFirstLetter.cpp:
+        (WebCore::RenderTreeBuilder::FirstLetter::createRenderers):
+
</ins><span class="cx"> 2019-03-21  Eric Carlson  <eric.carlson@apple.com>
</span><span class="cx"> 
</span><span class="cx">         Add UI process WebRTC runtime logging.
</span></span></pre></div>
<a id="trunkSourceWebCorerenderingupdatingRenderTreeBuilderFirstLettercpp"></a>
<div class="modfile"><h4>Modified: trunk/Source/WebCore/rendering/updating/RenderTreeBuilderFirstLetter.cpp (243330 => 243331)</h4>
<pre class="diff"><span>
<span class="info">--- trunk/Source/WebCore/rendering/updating/RenderTreeBuilderFirstLetter.cpp 2019-03-21 21:36:26 UTC (rev 243330)
+++ trunk/Source/WebCore/rendering/updating/RenderTreeBuilderFirstLetter.cpp    2019-03-21 21:43:14 UTC (rev 243331)
</span><span class="lines">@@ -217,9 +217,6 @@
</span><span class="cx">     newFirstLetter->initializeStyle();
</span><span class="cx">     newFirstLetter->setIsFirstLetter();
</span><span class="cx"> 
</span><del>-    auto& firstLetter = *newFirstLetter;
-    m_builder.attach(*firstLetterContainer, WTFMove(newFirstLetter), &currentTextChild);
-
</del><span class="cx">     // The original string is going to be either a generated content string or a DOM node's
</span><span class="cx">     // string. We want the original string before it got transformed in case first-letter has
</span><span class="cx">     // no text-transform or a different text-transform applied to it.
</span><span class="lines">@@ -253,6 +250,8 @@
</span><span class="cx"> 
</span><span class="cx">         auto* textNode = currentTextChild.textNode();
</span><span class="cx">         auto* beforeChild = currentTextChild.nextSibling();
</span><ins>+        auto inlineWrapperForDisplayContents = makeWeakPtr(currentTextChild.inlineWrapperForDisplayContents());
+        auto hasInlineWrapperForDisplayContents = inlineWrapperForDisplayContents.get();
</ins><span class="cx">         m_builder.destroy(currentTextChild);
</span><span class="cx"> 
</span><span class="cx">         // Construct a text fragment for the text after the first letter.
</span><span class="lines">@@ -265,13 +264,18 @@
</span><span class="cx">             newRemainingText = createRenderer<RenderTextFragment>(firstLetterBlock.document(), oldText, length, oldText.length() - length);
</span><span class="cx"> 
</span><span class="cx">         RenderTextFragment& remainingText = *newRemainingText;
</span><ins>+        ASSERT_UNUSED(hasInlineWrapperForDisplayContents, hasInlineWrapperForDisplayContents == inlineWrapperForDisplayContents.get());
+        remainingText.setInlineWrapperForDisplayContents(inlineWrapperForDisplayContents.get());
</ins><span class="cx">         m_builder.attach(*textContentParent, WTFMove(newRemainingText), beforeChild);
</span><ins>+
+        // FIXME: Make attach the final step so that we don't need to keep firstLetter around.
+        auto& firstLetter = *newFirstLetter;
</ins><span class="cx">         remainingText.setFirstLetter(firstLetter);
</span><span class="cx">         firstLetter.setFirstLetterRemainingText(remainingText);
</span><ins>+        m_builder.attach(*firstLetterContainer, WTFMove(newFirstLetter), &remainingText);
</ins><span class="cx"> 
</span><del>-        // construct text fragment for the first letter
</del><ins>+        // Construct text fragment for the first letter.
</ins><span class="cx">         auto letter = createRenderer<RenderTextFragment>(firstLetterBlock.document(), oldText, 0, length);
</span><del>-
</del><span class="cx">         m_builder.attach(firstLetter, WTFMove(letter));
</span><span class="cx">     }
</span><span class="cx"> }
</span></span></pre>
</div>
</div>

</body>
</html>