Changeset 233055 in webkit

Timestamp:

Jun 21, 2018 1:34:14 PM (6 years ago)

Author:

Alan Bujtas

Message:

Do not reuse generated inline renderer for the first letter.
​https://bugs.webkit.org/show_bug.cgi?id=186657
<rdar://problem/41157892>

Reviewed by Simon Fraser.

Source/WebCore:

When the first letter pseudo element is present, we construct a dedicated subtree for its content like this:

<div><span>foobar</span></div> -> with ::first-letter on the <div>
DIV RenderBlock

SPAN RenderInline

RenderInline (generated wrapper for the first letter content)

RenderText (fist letter content)

RenderText (remaining text content)

"display: contents" on the <span> forces us not to construct a RenderInline for the <span> (or just come up with a wrapper at best).
FirstLetter::createRenderers logic needs to check for such cases and pick the correct parent for the first letter subtree accordingly.

Test: fast/text/first-letter-with-display-contents-crash.html

imported/w3c/web-platform-tests/css/css-display/display-contents-first-letter-002.html is not crashing anymore either.

• rendering/updating/RenderTreeBuilderFirstLetter.cpp:

(WebCore::RenderTreeBuilder::FirstLetter::createRenderers):

LayoutTests:

• fast/text/first-letter-with-display-contents-crash-expected.txt: Added.
• fast/text/first-letter-with-display-contents-crash.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/fast/text/first-letter-with-display-contents-crash-expected.txt (added)
• LayoutTests/fast/text/first-letter-with-display-contents-crash.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/updating/RenderTreeBuilderFirstLetter.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-06-20  Zalan Bujtas  <zalan@apple.com>
+
+        Do not reuse generated inline renderer for the first letter.
+        https://bugs.webkit.org/show_bug.cgi?id=186657
+        <rdar://problem/41157892>
+
+        Reviewed by Simon Fraser.
+
+        * fast/text/first-letter-with-display-contents-crash-expected.txt: Added.
+        * fast/text/first-letter-with-display-contents-crash.html: Added.
+
 2018-06-21  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-06-20  Zalan Bujtas  <zalan@apple.com>
+
+        Do not reuse generated inline renderer for the first letter.
+        https://bugs.webkit.org/show_bug.cgi?id=186657
+        <rdar://problem/41157892>
+
+        Reviewed by Simon Fraser.
+
+        When the first letter pseudo element is present, we construct a dedicated subtree for its content like this:
+
+        <div><span>foobar</span></div> -> with ::first-letter on the <div>
+        DIV RenderBlock
+            SPAN RenderInline
+                RenderInline (generated wrapper for the first letter content)
+                    RenderText (fist letter content)
+                RenderText (remaining text content)
+
+        "display: contents" on the <span> forces us not to construct a RenderInline for the <span> (or just come up with a wrapper at best).
+        FirstLetter::createRenderers logic needs to check for such cases and pick the correct parent for the first letter subtree accordingly.
+
+        Test: fast/text/first-letter-with-display-contents-crash.html
+              imported/w3c/web-platform-tests/css/css-display/display-contents-first-letter-002.html is not crashing anymore either.
+
+        * rendering/updating/RenderTreeBuilderFirstLetter.cpp:
+        (WebCore::RenderTreeBuilder::FirstLetter::createRenderers):
+
 2018-06-21  Chris Dumez  <cdumez@apple.com>
 

trunk/Source/WebCore/rendering/updating/RenderTreeBuilderFirstLetter.cpp

@@ -203 +203 @@
 void RenderTreeBuilder::FirstLetter::createRenderers(RenderBlock& firstLetterBlock, RenderText& currentTextChild)
 {
-    RenderElement* firstLetterContainer = currentTextChild.parent();
+    RenderElement* textContentParent = currentTextChild.parent();
+    RenderElement* firstLetterContainer = nullptr;
+    if (auto* wrapperInlineForDisplayContents = currentTextChild.inlineWrapperForDisplayContents())
+        firstLetterContainer = wrapperInlineForDisplayContents->parent();
+    else
+        firstLetterContainer = textContentParent;
     auto pseudoStyle = styleForFirstLetter(firstLetterBlock, *firstLetterContainer);
     RenderPtr<RenderBoxModelObject> newFirstLetter;
@@ -261 +266 @@
 
         RenderTextFragment& remainingText = *newRemainingText;
-        m_builder.attach(*firstLetterContainer, WTFMove(newRemainingText), beforeChild);
+        m_builder.attach(*textContentParent, WTFMove(newRemainingText), beforeChild);
         remainingText.setFirstLetter(firstLetter);
         firstLetter.setFirstLetterRemainingText(remainingText);