Do not insert the first-letter anonymous container until after we've constructed...
authorzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Mar 2019 21:43:14 +0000 (21:43 +0000)
committerzalan@apple.com <zalan@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Mar 2019 21:43:14 +0000 (21:43 +0000)
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:

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@243331 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/TestExpectations
LayoutTests/fast/css/first-letter-and-float-crash-expected.txt [new file with mode: 0644]
LayoutTests/fast/css/first-letter-and-float-crash.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/updating/RenderTreeBuilderFirstLetter.cpp

index 7929d00..d841f6b 100644 (file)
@@ -1,3 +1,15 @@
+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:
+
 2019-03-21  Eric Carlson  <eric.carlson@apple.com>
 
         Add UI process WebRTC runtime logging.
index a0ad9c3..c71b29c 100644 (file)
@@ -3059,3 +3059,5 @@ imported/w3c/web-platform-tests/css/css-lists/counter-increment-inside-display-c
 imported/w3c/web-platform-tests/css/css-lists/counter-reset-inside-display-contents.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/css-lists/list-marker-with-lineheight-and-overflow-hidden-001.html [ ImageOnlyFailure ]
 imported/w3c/web-platform-tests/css/css-lists/list-with-image-display-changed-001.html [ ImageOnlyFailure ]
+
+[ Debug ] fast/css/first-letter-and-float-crash.html [ Skip ]
diff --git a/LayoutTests/fast/css/first-letter-and-float-crash-expected.txt b/LayoutTests/fast/css/first-letter-and-float-crash-expected.txt
new file mode 100644 (file)
index 0000000..9dc5b8f
--- /dev/null
@@ -0,0 +1 @@
+Pass if no crash
diff --git a/LayoutTests/fast/css/first-letter-and-float-crash.html b/LayoutTests/fast/css/first-letter-and-float-crash.html
new file mode 100644 (file)
index 0000000..dd27fe9
--- /dev/null
@@ -0,0 +1,11 @@
+<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>
\ No newline at end of file
index 15a4e61..b666833 100644 (file)
@@ -1,3 +1,19 @@
+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):
+
 2019-03-21  Eric Carlson  <eric.carlson@apple.com>
 
         Add UI process WebRTC runtime logging.
index 8fd021d..1eba71f 100644 (file)
@@ -217,9 +217,6 @@ void RenderTreeBuilder::FirstLetter::createRenderers(RenderBlock& firstLetterBlo
     newFirstLetter->initializeStyle();
     newFirstLetter->setIsFirstLetter();
 
-    auto& firstLetter = *newFirstLetter;
-    m_builder.attach(*firstLetterContainer, WTFMove(newFirstLetter), &currentTextChild);
-
     // The original string is going to be either a generated content string or a DOM node's
     // string. We want the original string before it got transformed in case first-letter has
     // no text-transform or a different text-transform applied to it.
@@ -253,6 +250,8 @@ void RenderTreeBuilder::FirstLetter::createRenderers(RenderBlock& firstLetterBlo
 
         auto* textNode = currentTextChild.textNode();
         auto* beforeChild = currentTextChild.nextSibling();
+        auto inlineWrapperForDisplayContents = makeWeakPtr(currentTextChild.inlineWrapperForDisplayContents());
+        auto hasInlineWrapperForDisplayContents = inlineWrapperForDisplayContents.get();
         m_builder.destroy(currentTextChild);
 
         // Construct a text fragment for the text after the first letter.
@@ -265,13 +264,18 @@ void RenderTreeBuilder::FirstLetter::createRenderers(RenderBlock& firstLetterBlo
             newRemainingText = createRenderer<RenderTextFragment>(firstLetterBlock.document(), oldText, length, oldText.length() - length);
 
         RenderTextFragment& remainingText = *newRemainingText;
+        ASSERT_UNUSED(hasInlineWrapperForDisplayContents, hasInlineWrapperForDisplayContents == inlineWrapperForDisplayContents.get());
+        remainingText.setInlineWrapperForDisplayContents(inlineWrapperForDisplayContents.get());
         m_builder.attach(*textContentParent, WTFMove(newRemainingText), beforeChild);
+
+        // FIXME: Make attach the final step so that we don't need to keep firstLetter around.
+        auto& firstLetter = *newFirstLetter;
         remainingText.setFirstLetter(firstLetter);
         firstLetter.setFirstLetterRemainingText(remainingText);
+        m_builder.attach(*firstLetterContainer, WTFMove(newFirstLetter), &remainingText);
 
-        // construct text fragment for the first letter
+        // Construct text fragment for the first letter.
         auto letter = createRenderer<RenderTextFragment>(firstLetterBlock.document(), oldText, 0, length);
-
         m_builder.attach(firstLetter, WTFMove(letter));
     }
 }