Text nodes with display:contents parent should render as if they were…

… wrapped in an unstyled <span>

https://bugs.webkit.org/show_bug.cgi?id=178332

Reviewed by Ryosuke Niwa.

Source/WebCore:

According to w3c/csswg-drafts#1118

    <div style="display:contents;color:green">text</div>

must result in green text even though div doesn't generate a box.

This patch implements the behavior by wrapping text renderers with display:contents parent element
in an anonymous inline box that receives its style by inheriting from the parent element.

* dom/Document.cpp:
(WebCore::Document::updateTextRenderer):
* rendering/RenderElement.cpp:
(WebCore::RenderElement::computeFirstLineStyle const):

    Synthesize the first line style in display:contents parent case.

* rendering/RenderObject.cpp:
(WebCore::findDestroyRootIncludingAnonymous):

    Factor into a function.

(WebCore::RenderObject::removeFromParentAndDestroyCleaningUpAnonymousWrappers):

    Get rid of the anonymous wrapper if it exists.

* rendering/RenderText.cpp:
(WebCore::inlineWrapperForDisplayContentsMap):
(WebCore::RenderText::RenderText):
(WebCore::RenderText::willBeDestroyed):
(WebCore::RenderText::inlineWrapperForDisplayContents):
(WebCore::RenderText::setInlineWrapperForDisplayContents):

    Add a weak member (implemented as a rare data map) for holding the wrapper pointer.

(WebCore::RenderText::findByDisplayContentsInlineWrapperCandidate):

    Helper to get the text renderer for a wrapper.

* rendering/RenderText.h:
* style/RenderTreeUpdater.cpp:
(WebCore::createTextRenderer):
(WebCore::RenderTreeUpdater::updateTextRenderer):

    Create the wrapper if needed.

* style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::resolveComposedTree):

    Compute the wrapper style by inheriting from the display:contents parent.

* style/StyleUpdate.h:
(WebCore::Style::TextUpdate::TextUpdate):

LayoutTests:

* TestExpectations: 10 more display:contents tests pass.


Canonical link: https://commits.webkit.org/194608@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@223514 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog

@@ -1,3 +1,12 @@
+2017-10-17  Antti Koivisto  <antti@apple.com>
+
+        Text nodes with display:contents parent should render as if they were wrapped in an unstyled <span>
+        https://bugs.webkit.org/show_bug.cgi?id=178332
+
+        Reviewed by Ryosuke Niwa.
+
+        * TestExpectations: 10 more display:contents tests pass.
+
 2017-10-17  Alicia Boya García  <aboya@igalia.com>
 
         [MSE][GStreamer] Insert parser elements in AppendPipeline when demuxing opus or Vorbis

LayoutTests/TestExpectations

@@ -1264,30 +1264,20 @@ imported/w3c/web-platform-tests/fetch/nosniff [ DumpJSConsoleLogInStdErr ]
 ########################################
 ### START OF display: contents failures
 
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-list-001.html [ ImageOnlyFailure ]
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-table-002-inline.html [ ImageOnlyFailure ]
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-inline-flex-001.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-before-after-first-letter-001.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-before-after-001.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-table-002-none.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-flex-003.html [ ImageOnlyFailure ]
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-table-001.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-before-after-002.html [ ImageOnlyFailure ]
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-table-002.html [ ImageOnlyFailure ]
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-inline-flex-001-none.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-table-001-inline.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-flex-002-none.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-flex-002.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-before-after-001.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-flow-root-001.html [ ImageOnlyFailure ]
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-inline-flex-001-inline.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-flex-003-none.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-table-001-none.html [ ImageOnlyFailure ]
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-state-change-001.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-flex-002-inline.html [ ImageOnlyFailure ]
 webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-flex-003-inline.html [ ImageOnlyFailure ]
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-list-001-inline.html [ ImageOnlyFailure ]
-webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-contents-dynamic-list-001-none.html [ ImageOnlyFailure ]
 
 ### END OF display: contents failures
 ########################################

Source/WebCore/ChangeLog

@@ -1,3 +1,63 @@
+2017-10-17  Antti Koivisto  <antti@apple.com>
+
+        Text nodes with display:contents parent should render as if they were wrapped in an unstyled <span>
+        https://bugs.webkit.org/show_bug.cgi?id=178332
+
+        Reviewed by Ryosuke Niwa.
+
+        According to https://github.com/w3c/csswg-drafts/issues/1118
+
+            <div style="display:contents;color:green">text</div>
+
+        must result in green text even though div doesn't generate a box.
+
+        This patch implements the behavior by wrapping text renderers with display:contents parent element
+        in an anonymous inline box that receives its style by inheriting from the parent element.
+
+        * dom/Document.cpp:
+        (WebCore::Document::updateTextRenderer):
+        * rendering/RenderElement.cpp:
+        (WebCore::RenderElement::computeFirstLineStyle const):
+
+            Synthesize the first line style in display:contents parent case.
+
+        * rendering/RenderObject.cpp:
+        (WebCore::findDestroyRootIncludingAnonymous):
+
+            Factor into a function.
+
+        (WebCore::RenderObject::removeFromParentAndDestroyCleaningUpAnonymousWrappers):
+
+            Get rid of the anonymous wrapper if it exists.
+
+        * rendering/RenderText.cpp:
+        (WebCore::inlineWrapperForDisplayContentsMap):
+        (WebCore::RenderText::RenderText):
+        (WebCore::RenderText::willBeDestroyed):
+        (WebCore::RenderText::inlineWrapperForDisplayContents):
+        (WebCore::RenderText::setInlineWrapperForDisplayContents):
+
+            Add a weak member (implemented as a rare data map) for holding the wrapper pointer.
+
+        (WebCore::RenderText::findByDisplayContentsInlineWrapperCandidate):
+
+            Helper to get the text renderer for a wrapper.
+
+        * rendering/RenderText.h:
+        * style/RenderTreeUpdater.cpp:
+        (WebCore::createTextRenderer):
+        (WebCore::RenderTreeUpdater::updateTextRenderer):
+
+            Create the wrapper if needed.
+
+        * style/StyleTreeResolver.cpp:
+        (WebCore::Style::TreeResolver::resolveComposedTree):
+
+            Compute the wrapper style by inheriting from the display:contents parent.
+
+        * style/StyleUpdate.h:
+        (WebCore::Style::TextUpdate::TextUpdate):
+
 2017-10-17  Alicia Boya García  <aboya@igalia.com>
 
         [MSE][GStreamer] Insert parser elements in AppendPipeline when demuxing opus or Vorbis

Source/WebCore/dom/Document.cpp

@@ -1871,7 +1871,7 @@ void Document::updateTextRenderer(Text& text, unsigned offsetOfReplacedText, uns
     SetForScope<bool> inRenderTreeUpdate(m_inRenderTreeUpdate, true);
 
     auto textUpdate = std::make_unique<Style::Update>(*this);
-    textUpdate->addText(text, { offsetOfReplacedText, lengthOfReplacedText });
+    textUpdate->addText(text, { offsetOfReplacedText, lengthOfReplacedText, std::nullopt });
 
     RenderTreeUpdater renderTreeUpdater(*this);
     renderTreeUpdater.commit(WTFMove(textUpdate));

Source/WebCore/rendering/RenderElement.cpp

@@ -217,12 +217,30 @@ std::unique_ptr<RenderStyle> RenderElement::computeFirstLineStyle() const
         return RenderStyle::clonePtr(*firstLineStyle);
     }
 
-    if (rendererForFirstLineStyle.isAnonymous() || !rendererForFirstLineStyle.isRenderInline())
+    if (!rendererForFirstLineStyle.isRenderInline())
         return nullptr;
 
     auto& parentStyle = rendererForFirstLineStyle.parent()->firstLineStyle();
     if (&parentStyle == &rendererForFirstLineStyle.parent()->style())
         return nullptr;
+
+    if (rendererForFirstLineStyle.isAnonymous()) {
+        auto* textRendererWithDisplayContentsParent = RenderText::findByDisplayContentsInlineWrapperCandidate(rendererForFirstLineStyle);
+        if (!textRendererWithDisplayContentsParent)
+            return nullptr;
+        auto* composedTreeParentElement = textRendererWithDisplayContentsParent->textNode()->parentElementInComposedTree();
+        if (!composedTreeParentElement)
+            return nullptr;
+
+        auto style = composedTreeParentElement->styleResolver().styleForElement(*composedTreeParentElement, &parentStyle).renderStyle;
+        ASSERT(style->display() == CONTENTS);
+
+        // We act as if there was an unstyled <span> around the text node. Only styling happens via inheritance.
+        auto firstLineStyle = RenderStyle::createPtr();
+        firstLineStyle->inheritFrom(*style);
+        return firstLineStyle;
+    }
+
     return rendererForFirstLineStyle.element()->styleResolver().styleForElement(*element(), &parentStyle).renderStyle;
 }
 

Source/WebCore/rendering/RenderObject.cpp

@@ -1455,15 +1455,11 @@ void RenderObject::willBeRemovedFromTree()
     parent()->setNeedsBoundariesUpdate();
 }
 
-void RenderObject::removeFromParentAndDestroyCleaningUpAnonymousWrappers()
+static RenderObject& findDestroyRootIncludingAnonymous(RenderObject& renderer)
 {
-    // If the tree is destroyed, there is no need for a clean-up phase.
-    if (renderTreeBeingDestroyed()) {
-        removeFromParentAndDestroy();
-        return;
-    }
+    auto* inlineWrapperForDisplayContents = is<RenderText>(renderer) ? downcast<RenderText>(renderer).inlineWrapperForDisplayContents() : nullptr;
 
-    auto* destroyRoot = this;
+    auto* destroyRoot = inlineWrapperForDisplayContents ? inlineWrapperForDisplayContents : &renderer;
     auto* destroyRootParent = destroyRoot->parent();
     while (destroyRootParent && destroyRootParent->isAnonymous()) {
         if (!destroyRootParent->isTableCell() && !destroyRootParent->isTableRow()
@@ -1475,11 +1471,23 @@ void RenderObject::removeFromParentAndDestroyCleaningUpAnonymousWrappers()
         destroyRoot = destroyRootParent;
         destroyRootParent = destroyRootParent->parent();
     }
+    return *destroyRoot;
+}
+
+void RenderObject::removeFromParentAndDestroyCleaningUpAnonymousWrappers()
+{
+    // If the tree is destroyed, there is no need for a clean-up phase.
+    if (renderTreeBeingDestroyed()) {
+        removeFromParentAndDestroy();
+        return;
+    }
+
+    auto& destroyRoot = findDestroyRootIncludingAnonymous(*this);
 
-    if (is<RenderTableRow>(*destroyRoot))
-        downcast<RenderTableRow>(*destroyRoot).collapseAndDestroyAnonymousSiblingRows();
+    if (is<RenderTableRow>(destroyRoot))
+        downcast<RenderTableRow>(destroyRoot).collapseAndDestroyAnonymousSiblingRows();
 
-    destroyRoot->removeFromParentAndDestroy();
+    destroyRoot.removeFromParentAndDestroy();
     // WARNING: |this| is deleted here.
 }
 

Source/WebCore/rendering/RenderText.cpp

@@ -132,6 +132,12 @@ static HashMap<const RenderText*, String>& originalTextMap()
     return map;
 }
 
+static HashMap<const RenderText*, WeakPtr<RenderInline>>& inlineWrapperForDisplayContentsMap()
+{
+    static NeverDestroyed<HashMap<const RenderText*, WeakPtr<RenderInline>>> map;
+    return map;
+}
+
 void makeCapitalized(String* string, UChar previous)
 {
     // FIXME: Need to change this to use u_strToTitle instead of u_totitle and to consider locale.
@@ -183,6 +189,7 @@ inline RenderText::RenderText(Node& node, const String& text)
     , m_knownToHaveNoOverflowAndNoFallbackFonts(false)
     , m_useBackslashAsYenSymbol(false)
     , m_originalTextDiffersFromRendered(false)
+    , m_hasInlineWrapperForDisplayContents(false)
 #if ENABLE(TEXT_AUTOSIZING)
     , m_candidateComputedTextSize(0)
 #endif
@@ -292,6 +299,8 @@ void RenderText::willBeDestroyed()
     if (m_originalTextDiffersFromRendered)
         originalTextMap().remove(this);
 
+    setInlineWrapperForDisplayContents(nullptr);
+
     RenderObject::willBeDestroyed();
 }
 
@@ -1737,4 +1746,42 @@ StringView RenderText::stringView(unsigned start, std::optional<unsigned> stop)
     return StringView(characters16() + start, destination - start);
 }
 
+RenderInline* RenderText::inlineWrapperForDisplayContents()
+{
+    ASSERT(m_hasInlineWrapperForDisplayContents == inlineWrapperForDisplayContentsMap().contains(this));
+
+    if (!m_hasInlineWrapperForDisplayContents)
+        return nullptr;
+    return inlineWrapperForDisplayContentsMap().get(this).get();
+}
+
+void RenderText::setInlineWrapperForDisplayContents(RenderInline* wrapper)
+{
+    ASSERT(m_hasInlineWrapperForDisplayContents == inlineWrapperForDisplayContentsMap().contains(this));
+
+    if (!wrapper) {
+        if (!m_hasInlineWrapperForDisplayContents)
+            return;
+        inlineWrapperForDisplayContentsMap().remove(this);
+        m_hasInlineWrapperForDisplayContents = false;
+        return;
+    }
+    inlineWrapperForDisplayContentsMap().add(this, makeWeakPtr(wrapper));
+    m_hasInlineWrapperForDisplayContents = true;
+}
+
+RenderText* RenderText::findByDisplayContentsInlineWrapperCandidate(RenderElement& renderer)
+{
+    auto* firstChild = renderer.firstChild();
+    if (!is<RenderText>(firstChild))
+        return nullptr;
+    auto& textRenderer = downcast<RenderText>(*firstChild);
+    if (textRenderer.inlineWrapperForDisplayContents() != &renderer)
+        return nullptr;
+    ASSERT(textRenderer.textNode());
+    ASSERT(renderer.firstChild() == renderer.lastChild());
+    return &textRenderer;
+
+}
+
 } // namespace WebCore

Source/WebCore/rendering/RenderText.h

@@ -179,6 +179,11 @@ class RenderText : public RenderObject {
 
     Vector<std::pair<unsigned, unsigned>> draggedContentRangesBetweenOffsets(unsigned startOffset, unsigned endOffset) const;
 
+    RenderInline* inlineWrapperForDisplayContents();
+    void setInlineWrapperForDisplayContents(RenderInline*);
+
+    static RenderText* findByDisplayContentsInlineWrapperCandidate(RenderElement&);
+
 protected:
     virtual void computePreferredLogicalWidths(float leadWidth);
     void willBeDestroyed() override;
@@ -233,6 +238,7 @@ class RenderText : public RenderObject {
     mutable unsigned m_knownToHaveNoOverflowAndNoFallbackFonts : 1;
     unsigned m_useBackslashAsYenSymbol : 1;
     unsigned m_originalTextDiffersFromRendered : 1;
+    unsigned m_hasInlineWrapperForDisplayContents : 1;
     unsigned m_canUseSimplifiedTextMeasuring : 1;
 
 #if ENABLE(TEXT_AUTOSIZING)

Source/WebCore/style/RenderTreeUpdater.cpp

@@ -38,6 +38,7 @@
 #include "PseudoElement.h"
 #include "RenderDescendantIterator.h"
 #include "RenderFullScreen.h"
+#include "RenderInline.h"
 #include "RenderListItem.h"
 #include "RenderTreeUpdaterFirstLetter.h"
 #include "RenderTreeUpdaterGeneratedContent.h"
@@ -428,26 +429,48 @@ static bool textRendererIsNeeded(const Text& textNode, const RenderTreePosition&
     return true;
 }
 
-static void createTextRenderer(Text& textNode, RenderTreePosition& renderTreePosition)
+static void createTextRenderer(Text& textNode, RenderTreePosition& renderTreePosition, const Style::TextUpdate* textUpdate)
 {
     ASSERT(!textNode.renderer());
 
-    auto newRenderer = textNode.createTextRenderer(renderTreePosition.parent().style());
-    ASSERT(newRenderer);
+    auto textRenderer = textNode.createTextRenderer(renderTreePosition.parent().style());
 
     renderTreePosition.computeNextSibling(textNode);
 
-    if (!renderTreePosition.canInsert(*newRenderer))
+    if (!renderTreePosition.canInsert(*textRenderer))
         return;
 
-    textNode.setRenderer(newRenderer.get());
-    renderTreePosition.insert(WTFMove(newRenderer));
+    textNode.setRenderer(textRenderer.get());
+
+    if (textUpdate && textUpdate->inheritedDisplayContentsStyle && *textUpdate->inheritedDisplayContentsStyle) {
+        // Wrap text renderer into anonymous inline so we can give it a style.
+        // This is to support "<div style='display:contents;color:green'>text</div>" type cases
+        auto newDisplayContentsAnonymousWrapper = createRenderer<RenderInline>(textNode.document(), RenderStyle::clone(**textUpdate->inheritedDisplayContentsStyle));
+        newDisplayContentsAnonymousWrapper->initializeStyle();
+        auto& displayContentsAnonymousWrapper = *newDisplayContentsAnonymousWrapper;
+        renderTreePosition.insert(WTFMove(newDisplayContentsAnonymousWrapper));
+
+        textRenderer->setInlineWrapperForDisplayContents(&displayContentsAnonymousWrapper);
+        displayContentsAnonymousWrapper.addChild(WTFMove(textRenderer));
+        return;
+    }
+
+    renderTreePosition.insert(WTFMove(textRenderer));
 }
 
 void RenderTreeUpdater::updateTextRenderer(Text& text, const Style::TextUpdate* textUpdate)
 {
     auto* existingRenderer = text.renderer();
     bool needsRenderer = textRendererIsNeeded(text, renderTreePosition());
+
+    if (existingRenderer && textUpdate && textUpdate->inheritedDisplayContentsStyle) {
+        if (existingRenderer->inlineWrapperForDisplayContents() || *textUpdate->inheritedDisplayContentsStyle) {
+            // FIXME: We could update without teardown.
+            tearDownRenderer(text);
+            existingRenderer = nullptr;
+        }
+    }
+
     if (existingRenderer) {
         if (needsRenderer) {
             if (textUpdate)
@@ -460,7 +483,7 @@ void RenderTreeUpdater::updateTextRenderer(Text& text, const Style::TextUpdate*
     }
     if (!needsRenderer)
         return;
-    createTextRenderer(text, renderTreePosition());
+    createTextRenderer(text, renderTreePosition(), textUpdate);
     invalidateWhitespaceOnlyTextSiblingsAfterAttachIfNeeded(text);
 }
 

Source/WebCore/style/StyleTreeResolver.cpp

@@ -372,6 +372,18 @@ static bool hasLoadingStylesheet(const Style::Scope& styleScope, const Element&
     return false;
 }
 
+static std::unique_ptr<RenderStyle> createInheritedDisplayContentsStyleIfNeeded(const RenderStyle& parentElementStyle, const RenderStyle* parentBoxStyle)
+{
+    if (parentElementStyle.display() != CONTENTS)
+        return nullptr;
+    if (parentBoxStyle && !parentBoxStyle->inheritedNotEqual(&parentElementStyle))
+        return nullptr;
+    // Compute style for imaginary unstyled <span> around the text node.
+    auto style = RenderStyle::createPtr();
+    style->inheritFrom(parentElementStyle);
+    return style;
+}
+
 void TreeResolver::resolveComposedTree()
 {
     ASSERT(m_parentStack.size() == 1);
@@ -396,8 +408,13 @@ void TreeResolver::resolveComposedTree()
 
         if (is<Text>(node)) {
             auto& text = downcast<Text>(node);
-            if (text.styleValidity() >= Validity::SubtreeAndRenderersInvalid && parent.change != Detach)
-                m_update->addText(text, parent.element, { });
+
+            if ((text.styleValidity() >= Validity::SubtreeAndRenderersInvalid && parent.change != Detach) || parent.style.display() == CONTENTS) {
+                TextUpdate textUpdate;
+                textUpdate.inheritedDisplayContentsStyle = createInheritedDisplayContentsStyleIfNeeded(parent.style, parentBoxStyle());
+
+                m_update->addText(text, parent.element, WTFMove(textUpdate));
+            }
 
             text.setHasValidStyle();
             it.traverseNextSkippingChildren();