Support ::before/::after pseudo elements with display:contents
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 Oct 2017 08:15:10 +0000 (08:15 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 21 Oct 2017 08:15:10 +0000 (08:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=178584

Reviewed by Ryosuke Niwa.

Source/WebCore:

This is cases like

::before { display:contents; content:'foo' }

* css/StyleResolver.cpp:
(WebCore::StyleResolver::adjustDisplayContentsStyle): Added.

    Allow display:contents on pseudo elements.
    Factor into function.

(WebCore::StyleResolver::adjustRenderStyle):
* dom/PseudoElement.h:

    Add a weak vector of content renderers.

* style/RenderTreePosition.h:
(WebCore::RenderTreePosition::moveToLastChild):

    Add a way to set a valid render tree position without a node.

* style/RenderTreeUpdaterGeneratedContent.cpp:
(WebCore::createContentRenderers):

    Take RenderTreePosition.

(WebCore::updateStyleForContentRenderers):

    Update based on the content renderer vector instead of doing a tree walk.

(WebCore::removeAndDestroyContentRenderers):

    Helper for destroying content renderers.

(WebCore::RenderTreeUpdater::GeneratedContent::updatePseudoElement):

    In the normal case create a render tree position for the pseudo element renderer and
    use RenderTreePosition::moveToLastChild to make it a valid position. (The existing
    RenderTreePosition interface didn't have way to move to positions in anonymous boxes)

    In the case of a non box generating display:contents pseudo element, use the current
    render tree position instead.

    Ensure that pseudo element renderers are destroyed before creating the new ones since in
    display:contents case they are not descendants of the pseudo renderer and don't get cleared
    automatically.

LayoutTests:

* TestExpectations: Enable imported/w3c/web-platform-tests/css/css-display-3/display-contents-before-after-002.html

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

LayoutTests/ChangeLog
LayoutTests/TestExpectations
Source/WebCore/ChangeLog
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/dom/PseudoElement.h
Source/WebCore/style/RenderTreePosition.h
Source/WebCore/style/RenderTreeUpdaterGeneratedContent.cpp

index 94e4dba..bc9db7b 100644 (file)
@@ -1,3 +1,12 @@
+2017-10-21  Antti Koivisto  <antti@apple.com>
+
+        Support ::before/::after pseudo elements with display:contents
+        https://bugs.webkit.org/show_bug.cgi?id=178584
+
+        Reviewed by Ryosuke Niwa.
+
+        * TestExpectations: Enable imported/w3c/web-platform-tests/css/css-display-3/display-contents-before-after-002.html
+
 2017-10-20  Joseph Pecoraro  <pecoraro@apple.com>
 
         Web Inspector: Support `async test() { ... }` in Inspector Test Suites
index 78db90c..fd04311 100644 (file)
@@ -1269,7 +1269,6 @@ webkit.org/b/157477 imported/w3c/web-platform-tests/css/css-display-3/display-co
 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-before-after-002.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 ]
index b5eb5c3..9c15d22 100644 (file)
@@ -1,3 +1,56 @@
+2017-10-21  Antti Koivisto  <antti@apple.com>
+
+        Support ::before/::after pseudo elements with display:contents
+        https://bugs.webkit.org/show_bug.cgi?id=178584
+
+        Reviewed by Ryosuke Niwa.
+
+        This is cases like
+
+        ::before { display:contents; content:'foo' }
+
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::adjustDisplayContentsStyle): Added.
+
+            Allow display:contents on pseudo elements.
+            Factor into function.
+
+        (WebCore::StyleResolver::adjustRenderStyle):
+        * dom/PseudoElement.h:
+
+            Add a weak vector of content renderers.
+
+        * style/RenderTreePosition.h:
+        (WebCore::RenderTreePosition::moveToLastChild):
+
+            Add a way to set a valid render tree position without a node.
+
+        * style/RenderTreeUpdaterGeneratedContent.cpp:
+        (WebCore::createContentRenderers):
+
+            Take RenderTreePosition.
+
+        (WebCore::updateStyleForContentRenderers):
+
+            Update based on the content renderer vector instead of doing a tree walk.
+
+        (WebCore::removeAndDestroyContentRenderers):
+
+            Helper for destroying content renderers.
+
+        (WebCore::RenderTreeUpdater::GeneratedContent::updatePseudoElement):
+
+            In the normal case create a render tree position for the pseudo element renderer and
+            use RenderTreePosition::moveToLastChild to make it a valid position. (The existing
+            RenderTreePosition interface didn't have way to move to positions in anonymous boxes)
+
+            In the case of a non box generating display:contents pseudo element, use the current
+            render tree position instead.
+
+            Ensure that pseudo element renderers are destroyed before creating the new ones since in
+            display:contents case they are not descendants of the pseudo renderer and don't get cleared
+            automatically.
+
 2017-10-20  Zalan Bujtas  <zalan@apple.com>
 
         [FrameView::layout cleanup] Use SetForScope to ensure layout state correctness
index e25bde4..09ae7b1 100644 (file)
@@ -789,6 +789,22 @@ static bool hasEffectiveDisplayNoneForDisplayContents(const Element& element)
     return tagNames.get().contains(element.localName());
 }
 
+static void adjustDisplayContentsStyle(RenderStyle& style, const Element* element)
+{
+    bool displayContentsEnabled = is<HTMLSlotElement>(element) || RuntimeEnabledFeatures::sharedFeatures().displayContentsEnabled();
+    if (!displayContentsEnabled) {
+        style.setDisplay(INLINE);
+        return;
+    }
+    if (!element) {
+        if (style.styleType() != BEFORE && style.styleType() != AFTER)
+            style.setDisplay(NONE);
+        return;
+    }
+    if (hasEffectiveDisplayNoneForDisplayContents(*element))
+        style.setDisplay(NONE);
+}
+
 void StyleResolver::adjustRenderStyle(RenderStyle& style, const RenderStyle& parentStyle, const RenderStyle* parentBoxStyle, const Element* element)
 {
     // If the composed tree parent has display:contents, the parent box style will be different from the parent style.
@@ -799,13 +815,8 @@ void StyleResolver::adjustRenderStyle(RenderStyle& style, const RenderStyle& par
     // Cache our original display.
     style.setOriginalDisplay(style.display());
 
-    if (style.display() == CONTENTS) {
-        bool elementSupportsDisplayContents = is<HTMLSlotElement>(element) || RuntimeEnabledFeatures::sharedFeatures().displayContentsEnabled();
-        if (!elementSupportsDisplayContents)
-            style.setDisplay(INLINE);
-        else if (!element || hasEffectiveDisplayNoneForDisplayContents(*element))
-            style.setDisplay(NONE);
-    }
+    if (style.display() == CONTENTS)
+        adjustDisplayContentsStyle(style, element);
 
     if (style.display() != NONE && style.display() != CONTENTS) {
         if (element) {
index 280950c..b6d76c9 100644 (file)
@@ -44,6 +44,8 @@ public:
     bool canStartSelection() const override { return false; }
     bool canContainRangeEndPoint() const override { return false; }
 
+    auto& contentRenderers() { return m_contentRenderers; }
+
     static String pseudoElementNameForEvents(PseudoId);
 
 private:
@@ -53,6 +55,7 @@ private:
 
     Element* m_hostElement;
     PseudoId m_pseudoId;
+    Vector<WeakPtr<RenderObject>> m_contentRenderers;
 };
 
 const QualifiedName& pseudoElementTagName();
index 66d3d2f..0dae691 100644 (file)
@@ -34,12 +34,6 @@ namespace WebCore {
 
 class RenderTreePosition {
 public:
-    explicit RenderTreePosition(RenderView& root)
-        : m_parent(root)
-        , m_hasValidNextSibling(true)
-    {
-    }
-    
     explicit RenderTreePosition(RenderElement& parent)
         : m_parent(parent)
     {
@@ -51,6 +45,7 @@ public:
     bool canInsert(RenderText&) const;
 
     void computeNextSibling(const Node&);
+    void moveToLastChild();
     void invalidateNextSibling() { m_hasValidNextSibling = false; }
     void invalidateNextSibling(const RenderObject&);
 
@@ -66,6 +61,12 @@ private:
 #endif
 };
 
+inline void RenderTreePosition::moveToLastChild()
+{
+    m_nextSibling = nullptr;
+    m_hasValidNextSibling = true;
+}
+
 inline bool RenderTreePosition::canInsert(RenderElement& renderer) const
 {
     ASSERT(!renderer.parent());
index a620a05..dcc178b 100644 (file)
@@ -69,27 +69,39 @@ void RenderTreeUpdater::GeneratedContent::updateQuotesUpTo(RenderQuote* lastQuot
     ASSERT(!lastQuote);
 }
 
-static void createContentRenderers(RenderElement& renderer)
+static void createContentRenderers(PseudoElement& pseudoElement, const RenderStyle& style, RenderTreePosition& renderTreePosition)
 {
-    auto& style = renderer.style();
     ASSERT(style.contentData());
 
     for (const ContentData* content = style.contentData(); content; content = content->next()) {
-        auto child = content->createContentRenderer(renderer.document(), style);
-        if (renderer.isChildAllowed(*child, style))
-            renderer.addChild(WTFMove(child));
+        auto child = content->createContentRenderer(renderTreePosition.parent().document(), style);
+        pseudoElement.contentRenderers().append(makeWeakPtr(*child));
+        if (renderTreePosition.parent().isChildAllowed(*child, style))
+            renderTreePosition.insert(WTFMove(child));
     }
 }
 
-static void updateStyleForContentRenderers(RenderElement& renderer)
+static void updateStyleForContentRenderers(PseudoElement& pseudoElement, const RenderStyle& style)
 {
-    for (auto* child = renderer.nextInPreOrder(&renderer); child; child = child->nextInPreOrder(&renderer)) {
+    for (auto& contentRenderer : pseudoElement.contentRenderers()) {
+        if (!contentRenderer)
+            continue;
         // We only manage the style for the generated content which must be images or text.
-        if (!is<RenderImage>(*child) && !is<RenderQuote>(*child))
+        if (!is<RenderImage>(*contentRenderer) && !is<RenderQuote>(*contentRenderer))
+            continue;
+        auto createdStyle = RenderStyle::createStyleInheritingFromPseudoStyle(style);
+        downcast<RenderElement>(*contentRenderer).setStyle(WTFMove(createdStyle));
+    }
+}
+
+static void removeAndDestroyContentRenderers(PseudoElement& pseudoElement)
+{
+    for (auto& contentRenderer : pseudoElement.contentRenderers()) {
+        if (!contentRenderer)
             continue;
-        auto createdStyle = RenderStyle::createStyleInheritingFromPseudoStyle(renderer.style());
-        downcast<RenderElement>(*child).setStyle(WTFMove(createdStyle));
+        contentRenderer->removeFromParentAndDestroy();
     }
+    pseudoElement.contentRenderers().clear();
 }
 
 void RenderTreeUpdater::GeneratedContent::updatePseudoElement(Element& current, const std::optional<Style::ElementUpdate>& update, PseudoId pseudoId)
@@ -101,6 +113,8 @@ void RenderTreeUpdater::GeneratedContent::updatePseudoElement(Element& current,
 
     if (!needsPseudoElement(update)) {
         if (pseudoElement) {
+            removeAndDestroyContentRenderers(*pseudoElement);
+
             if (pseudoId == BEFORE)
                 current.clearBeforePseudoElement();
             else
@@ -127,21 +141,30 @@ void RenderTreeUpdater::GeneratedContent::updatePseudoElement(Element& current,
 
     m_updater.updateElementRenderer(*pseudoElement, *update);
 
-    auto* pseudoRenderer = pseudoElement->renderer();
-    if (!pseudoRenderer)
+    auto* pseudoElementRenderer = pseudoElement->renderer();
+    if (!pseudoElementRenderer && update->style->display() != CONTENTS)
         return;
 
-    if (update->change == Style::Detach)
-        createContentRenderers(*pseudoRenderer);
-    else
-        updateStyleForContentRenderers(*pseudoRenderer);
+    auto renderTreePosition = pseudoElementRenderer ? RenderTreePosition(*pseudoElementRenderer) : m_updater.renderTreePosition();
+
+    if (update->change == Style::Detach) {
+        removeAndDestroyContentRenderers(*pseudoElement);
+
+        if (pseudoElementRenderer)
+            renderTreePosition.moveToLastChild();
+        else
+            renderTreePosition.computeNextSibling(*pseudoElement);
+
+        createContentRenderers(*pseudoElement, *update->style, renderTreePosition);
+    } else
+        updateStyleForContentRenderers(*pseudoElement, *update->style);
 
     if (m_updater.renderView().hasQuotesNeedingUpdate()) {
-        for (auto& child : descendantsOfType<RenderQuote>(*pseudoRenderer))
+        for (auto& child : descendantsOfType<RenderQuote>(renderTreePosition.parent()))
             updateQuotesUpTo(&child);
     }
-    if (is<RenderListItem>(*pseudoRenderer))
-        ListItem::updateMarker(downcast<RenderListItem>(*pseudoRenderer));
+    if (is<RenderListItem>(renderTreePosition.parent()))
+        ListItem::updateMarker(downcast<RenderListItem>(renderTreePosition.parent()));
 }
 
 bool RenderTreeUpdater::GeneratedContent::needsPseudoElement(const std::optional<Style::ElementUpdate>& update)