::before/::after elements not filling their grid cell when container has display...
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Oct 2019 17:51:58 +0000 (17:51 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 30 Oct 2019 17:51:58 +0000 (17:51 +0000)
https://bugs.webkit.org/show_bug.cgi?id=193567

Reviewed by Simon Fraser.

Source/WebCore:

Test: fast/css/display-contents-before-after-grid.html

We were not providing parent box style when resolving :before/:after pseudo elements. Because of this we
failed to blockify the pseudo elements when their host element had 'display:contents' and the parent
box was a grid.

Original test case by Michał Gołębiowski-Owczarek.

* css/StyleResolver.cpp:
(WebCore::StyleResolver::pseudoStyleForElement):

Pass the parent box style.

* css/StyleResolver.h:
* style/StyleTreeResolver.cpp:
(WebCore::Style::TreeResolver::resolvePseudoStyle):
(WebCore::Style::TreeResolver::parentBoxStyle const):
(WebCore::Style::TreeResolver::parentBoxStyleForPseudo const):

If the element has 'display:contents', look for style of its ancestors.

* style/StyleTreeResolver.h:

LayoutTests:

* fast/css/display-contents-before-after-grid-expected.html: Added.
* fast/css/display-contents-before-after-grid.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/css/display-contents-before-after-grid-expected.html [new file with mode: 0644]
LayoutTests/fast/css/display-contents-before-after-grid.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/StyleResolver.cpp
Source/WebCore/css/StyleResolver.h
Source/WebCore/style/StyleTreeResolver.cpp
Source/WebCore/style/StyleTreeResolver.h

index 6684274..7565b84 100644 (file)
@@ -1,5 +1,15 @@
 2019-10-30  Antti Koivisto  <antti@apple.com>
 
+        ::before/::after elements not filling their grid cell when container has display: contents
+        https://bugs.webkit.org/show_bug.cgi?id=193567
+
+        Reviewed by Simon Fraser.
+
+        * fast/css/display-contents-before-after-grid-expected.html: Added.
+        * fast/css/display-contents-before-after-grid.html: Added.
+
+2019-10-30  Antti Koivisto  <antti@apple.com>
+
         Update css/css-display web platform tests
         https://bugs.webkit.org/show_bug.cgi?id=203607
 
diff --git a/LayoutTests/fast/css/display-contents-before-after-grid-expected.html b/LayoutTests/fast/css/display-contents-before-after-grid-expected.html
new file mode 100644 (file)
index 0000000..7fed959
--- /dev/null
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>CSS Grid, display: contents &amp; ::before/::after</title>
+    <style>
+        .grid {
+            display: grid;
+            grid-template-columns: repeat(5, 150px);
+        }
+
+        span {
+            width: 100px;
+            height: 100px;
+            background-color: green;
+        }
+    </style>
+</head>
+<body>
+    <div class="grid">
+        <span>before</span>
+        <span>after</span>
+        <span>span</span>
+        <span>before</span>
+        <span>after</span>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/fast/css/display-contents-before-after-grid.html b/LayoutTests/fast/css/display-contents-before-after-grid.html
new file mode 100644 (file)
index 0000000..2b89184
--- /dev/null
@@ -0,0 +1,43 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <title>CSS Grid, display: contents &amp; ::before/::after</title>
+    <style>
+        .grid {
+            display: grid;
+            grid-template-columns: repeat(5, 150px);
+        }
+
+        .replaced {
+            display: contents;
+        }
+
+        .replaced::before {
+            content: "before";
+        }
+
+        .replaced::after {
+            content: "after";
+        }
+
+        .replaced-container {
+            display: contents;
+        }
+
+        .replaced::before, .replaced::after, span {
+            width: 100px;
+            height: 100px;
+            background-color: green;
+        }
+    </style>
+</head>
+<body>
+    <div class="grid">
+        <div class="replaced"></div>
+        <span>span</span>
+        <div class="replaced-container">
+            <div class="replaced"></div>
+        </div>
+    </div>
+</body>
+</html>
index 5e380a2..170a6d9 100644 (file)
@@ -1,3 +1,33 @@
+2019-10-30  Antti Koivisto  <antti@apple.com>
+
+        ::before/::after elements not filling their grid cell when container has display: contents
+        https://bugs.webkit.org/show_bug.cgi?id=193567
+
+        Reviewed by Simon Fraser.
+
+        Test: fast/css/display-contents-before-after-grid.html
+
+        We were not providing parent box style when resolving :before/:after pseudo elements. Because of this we
+        failed to blockify the pseudo elements when their host element had 'display:contents' and the parent
+        box was a grid.
+
+        Original test case by Michał Gołębiowski-Owczarek.
+
+        * css/StyleResolver.cpp:
+        (WebCore::StyleResolver::pseudoStyleForElement):
+
+        Pass the parent box style.
+
+        * css/StyleResolver.h:
+        * style/StyleTreeResolver.cpp:
+        (WebCore::Style::TreeResolver::resolvePseudoStyle):
+        (WebCore::Style::TreeResolver::parentBoxStyle const):
+        (WebCore::Style::TreeResolver::parentBoxStyleForPseudo const):
+
+        If the element has 'display:contents', look for style of its ancestors.
+
+        * style/StyleTreeResolver.h:
+
 2019-10-30  Dirk Schulze  <krit@webkit.org>
 
         [css-masking] Unprefix -webkit-clip-path
index f6c7ffa..5867e3e 100644 (file)
@@ -499,7 +499,7 @@ void StyleResolver::keyframeStylesForAnimation(const Element& element, const Ren
     }
 }
 
-std::unique_ptr<RenderStyle> StyleResolver::pseudoStyleForElement(const Element& element, const PseudoStyleRequest& pseudoStyleRequest, const RenderStyle& parentStyle, const SelectorFilter* selectorFilter)
+std::unique_ptr<RenderStyle> StyleResolver::pseudoStyleForElement(const Element& element, const PseudoStyleRequest& pseudoStyleRequest, const RenderStyle& parentStyle, const RenderStyle* parentBoxStyle, const SelectorFilter* selectorFilter)
 {
     m_state = State(element, &parentStyle, m_overrideDocumentElementStyle, selectorFilter);
 
@@ -537,7 +537,7 @@ std::unique_ptr<RenderStyle> StyleResolver::pseudoStyleForElement(const Element&
     applyMatchedProperties(collector.matchResult(), element);
 
     // Clean up our style object's display and text decorations (among other fixups).
-    adjustRenderStyle(*state.style(), *m_state.parentStyle(), nullptr, nullptr);
+    adjustRenderStyle(*state.style(), *m_state.parentStyle(), parentBoxStyle, nullptr);
 
     if (state.style()->hasViewportUnits())
         document().setHasStyleWithViewportUnits();
index 57c98a1..270041f 100644 (file)
@@ -116,7 +116,7 @@ public:
 
     void keyframeStylesForAnimation(const Element&, const RenderStyle*, KeyframeList&);
 
-    std::unique_ptr<RenderStyle> pseudoStyleForElement(const Element&, const PseudoStyleRequest&, const RenderStyle& parentStyle, const SelectorFilter* = nullptr);
+    std::unique_ptr<RenderStyle> pseudoStyleForElement(const Element&, const PseudoStyleRequest&, const RenderStyle& parentStyle, const RenderStyle* parentBoxStyle = nullptr, const SelectorFilter* = nullptr);
 
     std::unique_ptr<RenderStyle> styleForPage(int pageIndex);
     std::unique_ptr<RenderStyle> defaultStyleForElement();
index 02a0e37..bbeff87 100644 (file)
@@ -254,7 +254,7 @@ ElementUpdate TreeResolver::resolvePseudoStyle(Element& element, const ElementUp
     if (!elementUpdate.style->hasPseudoStyle(pseudoId))
         return { };
 
-    auto pseudoStyle = scope().styleResolver.pseudoStyleForElement(element, { pseudoId }, *elementUpdate.style, &scope().selectorFilter);
+    auto pseudoStyle = scope().styleResolver.pseudoStyleForElement(element, { pseudoId }, *elementUpdate.style, parentBoxStyleForPseudo(elementUpdate), &scope().selectorFilter);
     if (!pseudoElementRendererIsNeeded(pseudoStyle.get()))
         return { };
 
@@ -274,8 +274,8 @@ ElementUpdate TreeResolver::resolvePseudoStyle(Element& element, const ElementUp
 const RenderStyle* TreeResolver::parentBoxStyle() const
 {
     // 'display: contents' doesn't generate boxes.
-    for (unsigned i = m_parentStack.size(); i; --i) {
-        auto& parent = m_parentStack[i - 1];
+    for (auto i = m_parentStack.size(); i--;) {
+        auto& parent = m_parentStack[i];
         if (parent.style.display() == DisplayType::None)
             return nullptr;
         if (parent.style.display() != DisplayType::Contents)
@@ -285,6 +285,18 @@ const RenderStyle* TreeResolver::parentBoxStyle() const
     return nullptr;
 }
 
+const RenderStyle* TreeResolver::parentBoxStyleForPseudo(const ElementUpdate& elementUpdate) const
+{
+    switch (elementUpdate.style->display()) {
+    case DisplayType::None:
+        return nullptr;
+    case DisplayType::Contents:
+        return parentBoxStyle();
+    default:
+        return elementUpdate.style.get();
+    }
+}
+
 ElementUpdate TreeResolver::createAnimatedElementUpdate(std::unique_ptr<RenderStyle> newStyle, Element& element, Change parentChange)
 {
     auto* oldStyle = element.renderOrDisplayContentsStyle();
index 899a9f6..d946100 100644 (file)
@@ -96,6 +96,7 @@ private:
     void popParentsToDepth(unsigned depth);
 
     const RenderStyle* parentBoxStyle() const;
+    const RenderStyle* parentBoxStyleForPseudo(const ElementUpdate&) const;
 
     Document& m_document;
     std::unique_ptr<RenderStyle> m_documentElementStyle;