Updating class name doesn't update the slotted content's style
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Nov 2016 23:08:34 +0000 (23:08 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 11 Nov 2016 23:08:34 +0000 (23:08 +0000)
https://bugs.webkit.org/show_bug.cgi?id=164577
<rdar://problem/29205873>

Reviewed by Ryosuke Niwa.

Source/WebCore:

Test: fast/shadow-dom/css-scoping-slotted-invalidation.html

Teach style invalidation code for attribute/class/id mutations about slotted rules.

* dom/ShadowRoot.cpp:
(WebCore::assignedShadowRootsIfSlotted):

    Helper to find all assigned shadow roots (there may be more than one if slots are assigned to slots).

* dom/ShadowRoot.h:
* style/AttributeChangeInvalidation.cpp:
(WebCore::Style::mayBeAffectedByAttributeChange):
(WebCore::Style::mayBeAffectedByHostRules):
(WebCore::Style::mayBeAffectedBySlottedRules):
(WebCore::Style::AttributeChangeInvalidation::invalidateStyle):
(WebCore::Style::mayBeAffectedByHostStyle): Deleted.
* style/ClassChangeInvalidation.cpp:
(WebCore::Style::mayBeAffectedByHostRules):
(WebCore::Style::mayBeAffectedBySlottedRules):
(WebCore::Style::ClassChangeInvalidation::invalidateStyle):
(WebCore::Style::mayBeAffectedByHostStyle): Deleted.
* style/ClassChangeInvalidation.h:
* style/IdChangeInvalidation.cpp:
(WebCore::Style::mayBeAffectedByHostRules):
(WebCore::Style::mayBeAffectedBySlottedRules):
(WebCore::Style::IdChangeInvalidation::invalidateStyle):
(WebCore::Style::mayBeAffectedByHostStyle): Deleted.
* style/StyleSharingResolver.cpp:
(WebCore::Style::SharingResolver::canShareStyleWithElement):

    Fix a bug in style sharing where we were checking wrong element for host rules.
    Tested by the included test too (the last empty div).

LayoutTests:

* fast/shadow-dom/css-scoping-slotted-invalidation-expected.html: Added.
* fast/shadow-dom/css-scoping-slotted-invalidation.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/shadow-dom/css-scoping-slotted-invalidation-expected.html [new file with mode: 0644]
LayoutTests/fast/shadow-dom/css-scoping-slotted-invalidation.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/dom/ShadowRoot.cpp
Source/WebCore/dom/ShadowRoot.h
Source/WebCore/style/AttributeChangeInvalidation.cpp
Source/WebCore/style/ClassChangeInvalidation.cpp
Source/WebCore/style/ClassChangeInvalidation.h
Source/WebCore/style/IdChangeInvalidation.cpp
Source/WebCore/style/StyleSharingResolver.cpp

index 6171e7b..a3e7d9a 100644 (file)
@@ -1,3 +1,14 @@
+2016-11-11  Antti Koivisto  <antti@apple.com>
+
+        Updating class name doesn't update the slotted content's style
+        https://bugs.webkit.org/show_bug.cgi?id=164577
+        <rdar://problem/29205873>
+
+        Reviewed by Ryosuke Niwa.
+
+        * fast/shadow-dom/css-scoping-slotted-invalidation-expected.html: Added.
+        * fast/shadow-dom/css-scoping-slotted-invalidation.html: Added.
+
 2016-11-11  Chris Dumez  <cdumez@apple.com>
 
         WorkerGlobalScope's indexedDB property should be on the prototype, not the instance
diff --git a/LayoutTests/fast/shadow-dom/css-scoping-slotted-invalidation-expected.html b/LayoutTests/fast/shadow-dom/css-scoping-slotted-invalidation-expected.html
new file mode 100644 (file)
index 0000000..eda713a
--- /dev/null
@@ -0,0 +1,7 @@
+<!DOCTYPE html>
+<html>
+    <body>
+        <p>Test passes if you see a single 100px by 100px green box below.</p>
+        <div style="width: 100px; height: 100px; background: green;"></div>
+    </body>
+</html>
diff --git a/LayoutTests/fast/shadow-dom/css-scoping-slotted-invalidation.html b/LayoutTests/fast/shadow-dom/css-scoping-slotted-invalidation.html
new file mode 100644 (file)
index 0000000..395b62f
--- /dev/null
@@ -0,0 +1,89 @@
+<!DOCTYPE html>
+<body>
+<p>Test passes if you see a single 100px by 100px green box below.</p>
+<div id="t1">
+    <span>FAIL</span>
+</div>
+<div id="t2">
+    <span slot="second-slot">FAIL</span>
+</div>
+<div id="t3">
+    <span>FAIL</span>
+</div>
+<div id="t4">
+    <span>FAIL</span>
+</div>
+<div id="t5">
+    <span selected>FAIL</span>
+</div>
+<div>
+</div>
+<script>
+
+function attachShadow(host, selector)
+{
+    const shadow = host.attachShadow({mode: 'closed'});
+    shadow.innerHTML = `
+    <style>
+    :host {
+        width: 100px;
+        height: 20px;
+        background: green;
+        color: red;
+    }
+    ${selector} {
+        color: green;
+    }
+    </style>
+    <slot></slot>
+    <slot name="second-slot"></slot>`;
+    return shadow;
+}
+
+{
+    const host = document.querySelector('#t1');
+    attachShadow(host, '::slotted(.selected)');
+    const span = host.querySelector('span');
+    getComputedStyle(span).backgroundColor;
+    span.className = 'selected';
+}
+
+{
+    const host = document.querySelector('#t2');
+    attachShadow(host, '[name=second-slot]::slotted(#selected)');
+    const span = host.querySelector('span');
+    getComputedStyle(span).backgroundColor
+    span.id = 'selected';
+}
+
+{
+    const host = document.querySelector('#t3');
+    attachShadow(host, '::slotted([selected])');
+    const span = host.querySelector('span');
+    getComputedStyle(span).backgroundColor;
+    span.setAttribute('selected', 'selected');
+}
+
+{
+    const host = document.querySelector('#t4');
+    const shadow = host.attachShadow({mode: 'closed'});
+    shadow.innerHTML = `<div><slot></slot></div>`;
+    const deepHost = shadow.querySelector('div');
+    attachShadow(deepHost, '::slotted(.selected)');
+    const span = host.querySelector('span');
+    getComputedStyle(span).backgroundColor;
+    span.className = 'selected';
+}
+
+{
+    const host = document.querySelector('#t5');
+    const shadow = host.attachShadow({mode: 'closed'});
+    shadow.innerHTML = `<div><slot slot="second-slot"></slot></div>`;
+    const deepHost = shadow.querySelector('div');
+    attachShadow(deepHost, '[name=second-slot]::slotted(:not([selected]))');
+    const span = host.querySelector('span');
+    getComputedStyle(span).backgroundColor;
+    span.removeAttribute('selected');
+}
+
+</script>
index e9e7d64..87a394e 100644 (file)
@@ -1,3 +1,44 @@
+2016-11-11  Antti Koivisto  <antti@apple.com>
+
+        Updating class name doesn't update the slotted content's style
+        https://bugs.webkit.org/show_bug.cgi?id=164577
+        <rdar://problem/29205873>
+
+        Reviewed by Ryosuke Niwa.
+
+        Test: fast/shadow-dom/css-scoping-slotted-invalidation.html
+
+        Teach style invalidation code for attribute/class/id mutations about slotted rules.
+
+        * dom/ShadowRoot.cpp:
+        (WebCore::assignedShadowRootsIfSlotted):
+
+            Helper to find all assigned shadow roots (there may be more than one if slots are assigned to slots).
+
+        * dom/ShadowRoot.h:
+        * style/AttributeChangeInvalidation.cpp:
+        (WebCore::Style::mayBeAffectedByAttributeChange):
+        (WebCore::Style::mayBeAffectedByHostRules):
+        (WebCore::Style::mayBeAffectedBySlottedRules):
+        (WebCore::Style::AttributeChangeInvalidation::invalidateStyle):
+        (WebCore::Style::mayBeAffectedByHostStyle): Deleted.
+        * style/ClassChangeInvalidation.cpp:
+        (WebCore::Style::mayBeAffectedByHostRules):
+        (WebCore::Style::mayBeAffectedBySlottedRules):
+        (WebCore::Style::ClassChangeInvalidation::invalidateStyle):
+        (WebCore::Style::mayBeAffectedByHostStyle): Deleted.
+        * style/ClassChangeInvalidation.h:
+        * style/IdChangeInvalidation.cpp:
+        (WebCore::Style::mayBeAffectedByHostRules):
+        (WebCore::Style::mayBeAffectedBySlottedRules):
+        (WebCore::Style::IdChangeInvalidation::invalidateStyle):
+        (WebCore::Style::mayBeAffectedByHostStyle): Deleted.
+        * style/StyleSharingResolver.cpp:
+        (WebCore::Style::SharingResolver::canShareStyleWithElement):
+
+            Fix a bug in style sharing where we were checking wrong element for host rules.
+            Tested by the included test too (the last empty div).
+
 2016-11-11  Dave Hyatt  <hyatt@apple.com>
 
         [CSS Parser] Support the spring animation timing function
index a3459ba..36ee86c 100644 (file)
@@ -31,6 +31,7 @@
 #include "CSSStyleSheet.h"
 #include "ElementTraversal.h"
 #include "ExceptionCode.h"
+#include "HTMLSlotElement.h"
 #include "RenderElement.h"
 #include "RuntimeEnabledFeatures.h"
 #include "SlotAssignment.h"
@@ -183,5 +184,14 @@ const Vector<Node*>* ShadowRoot::assignedNodesForSlot(const HTMLSlotElement& slo
     return m_slotAssignment->assignedNodesForSlot(slot, *this);
 }
 
+Vector<ShadowRoot*> assignedShadowRootsIfSlotted(const Node& node)
+{
+    Vector<ShadowRoot*> result;
+    for (auto* slot = node.assignedSlot(); slot; slot = slot->assignedSlot()) {
+        ASSERT(slot->containingShadowRoot());
+        result.append(slot->containingShadowRoot());
+    }
+    return result;
+}
 
 }
index d4e5e54..0119b8b 100644 (file)
@@ -133,6 +133,8 @@ inline bool hasShadowRootParent(const Node& node)
     return node.parentNode() && node.parentNode()->isShadowRoot();
 }
 
+Vector<ShadowRoot*> assignedShadowRootsIfSlotted(const Node&);
+
 } // namespace WebCore
 
 SPECIALIZE_TYPE_TRAITS_BEGIN(WebCore::ShadowRoot)
index 5d09aad..d1c3630 100644 (file)
 namespace WebCore {
 namespace Style {
 
-static bool mayBeAffectedByHostStyle(ShadowRoot& shadowRoot, bool isHTML, const QualifiedName& attributeName)
+static bool mayBeAffectedByAttributeChange(DocumentRuleSets& ruleSets, bool isHTML, const QualifiedName& attributeName)
 {
-    auto& shadowRuleSets = shadowRoot.styleScope().resolver().ruleSets();
+    auto& nameSet = isHTML ? ruleSets.features().attributeCanonicalLocalNamesInRules : ruleSets.features().attributeLocalNamesInRules;
+    return nameSet.contains(attributeName.localName().impl());
+}
+
+static bool mayBeAffectedByHostRules(const Element& element, const QualifiedName& attributeName)
+{
+    auto* shadowRoot = element.shadowRoot();
+    if (!shadowRoot)
+        return false;
+    auto& shadowRuleSets = shadowRoot->styleScope().resolver().ruleSets();
     if (shadowRuleSets.authorStyle().hostPseudoClassRules().isEmpty())
         return false;
 
-    auto& nameSet = isHTML ? shadowRuleSets.features().attributeCanonicalLocalNamesInRules : shadowRuleSets.features().attributeLocalNamesInRules;
-    return nameSet.contains(attributeName.localName().impl());
+    return mayBeAffectedByAttributeChange(shadowRuleSets, element.isHTMLElement(), attributeName);
+}
+
+static bool mayBeAffectedBySlottedRules(const Element& element, const QualifiedName& attributeName)
+{
+    for (auto* shadowRoot : assignedShadowRootsIfSlotted(element)) {
+        auto& ruleSets = shadowRoot->styleScope().resolver().ruleSets();
+        if (ruleSets.authorStyle().slottedPseudoElementRules().isEmpty())
+            continue;
+        if (mayBeAffectedByAttributeChange(ruleSets, element.isHTMLElement(), attributeName))
+            return true;
+    }
+    return false;
 }
 
 void AttributeChangeInvalidation::invalidateStyle(const QualifiedName& attributeName, const AtomicString& oldValue, const AtomicString& newValue)
@@ -54,12 +74,9 @@ void AttributeChangeInvalidation::invalidateStyle(const QualifiedName& attribute
     auto& ruleSets = m_element.styleResolver().ruleSets();
     bool isHTML = m_element.isHTMLElement();
 
-    auto& nameSet = isHTML ? ruleSets.features().attributeCanonicalLocalNamesInRules : ruleSets.features().attributeLocalNamesInRules;
-    bool mayAffectStyle = nameSet.contains(attributeName.localName().impl());
-
-    auto* shadowRoot = m_element.shadowRoot();
-    if (!mayAffectStyle && shadowRoot && mayBeAffectedByHostStyle(*shadowRoot, isHTML, attributeName))
-        mayAffectStyle = true;
+    bool mayAffectStyle = mayBeAffectedByAttributeChange(ruleSets, isHTML, attributeName)
+        || mayBeAffectedByHostRules(m_element, attributeName)
+        || mayBeAffectedBySlottedRules(m_element, attributeName);
 
     if (!mayAffectStyle)
         return;
index c3c6dfa..e25fc49 100644 (file)
@@ -86,28 +86,41 @@ static ClassChangeVector computeClassChange(const SpaceSplitString& oldClasses,
     return changedClasses;
 }
 
-static bool mayBeAffectedByHostStyle(ShadowRoot& shadowRoot, AtomicStringImpl* changedClass)
+static bool mayBeAffectedByHostRules(ShadowRoot* shadowRoot, AtomicStringImpl* changedClass)
 {
-    auto& shadowRuleSets = shadowRoot.styleScope().resolver().ruleSets();
+    if (!shadowRoot)
+        return false;
+    auto& shadowRuleSets = shadowRoot->styleScope().resolver().ruleSets();
     if (shadowRuleSets.authorStyle().hostPseudoClassRules().isEmpty())
         return false;
     return shadowRuleSets.features().classesInRules.contains(changedClass);
 }
 
+static bool mayBeAffectedBySlottedRules(const Vector<ShadowRoot*>& assignedShadowRoots, AtomicStringImpl* changedClass)
+{
+    for (auto& assignedShadowRoot : assignedShadowRoots) {
+        auto& ruleSets = assignedShadowRoot->styleScope().resolver().ruleSets();
+        if (ruleSets.authorStyle().slottedPseudoElementRules().isEmpty())
+            continue;
+        if (ruleSets.features().classesInRules.contains(changedClass))
+            return true;
+    }
+    return false;
+}
+
 void ClassChangeInvalidation::invalidateStyle(const SpaceSplitString& oldClasses, const SpaceSplitString& newClasses)
 {
     auto changedClasses = computeClassChange(oldClasses, newClasses);
 
     auto& ruleSets = m_element.styleResolver().ruleSets();
     auto* shadowRoot = m_element.shadowRoot();
+    auto assignedShadowRoots = assignedShadowRootsIfSlotted(m_element);
 
     ClassChangeVector changedClassesAffectingStyle;
     for (auto* changedClass : changedClasses) {
-        bool mayAffectStyle = ruleSets.features().classesInRules.contains(changedClass);
-
-        if (!mayAffectStyle && shadowRoot && mayBeAffectedByHostStyle(*shadowRoot, changedClass))
-            mayAffectStyle = true;
-
+        bool mayAffectStyle = ruleSets.features().classesInRules.contains(changedClass)
+            || mayBeAffectedByHostRules(shadowRoot, changedClass)
+            || mayBeAffectedBySlottedRules(assignedShadowRoots, changedClass);
         if (mayAffectStyle)
             changedClassesAffectingStyle.append(changedClass);
     };
index 6d44b18..8550f64 100644 (file)
@@ -69,7 +69,7 @@ inline ClassChangeInvalidation::~ClassChangeInvalidation()
         return;
     invalidateDescendantStyle();
 }
-    
+
 }
 }
 
index 0b010a2..17094fd 100644 (file)
 namespace WebCore {
 namespace Style {
 
-static bool mayBeAffectedByHostStyle(ShadowRoot& shadowRoot, const AtomicString& changedId)
+static bool mayBeAffectedByHostRules(const Element& element, const AtomicString& changedId)
 {
-    auto& shadowRuleSets = shadowRoot.styleScope().resolver().ruleSets();
+    auto* shadowRoot = element.shadowRoot();
+    if (!shadowRoot)
+        return false;
+    auto& shadowRuleSets = shadowRoot->styleScope().resolver().ruleSets();
     if (shadowRuleSets.authorStyle().hostPseudoClassRules().isEmpty())
         return false;
-
     return shadowRuleSets.features().idsInRules.contains(changedId.impl());
 }
 
+static bool mayBeAffectedBySlottedRules(const Element& element, const AtomicString& changedId)
+{
+    for (auto* shadowRoot : assignedShadowRootsIfSlotted(element)) {
+        auto& ruleSets = shadowRoot->styleScope().resolver().ruleSets();
+        if (ruleSets.authorStyle().slottedPseudoElementRules().isEmpty())
+            continue;
+        if (ruleSets.features().idsInRules.contains(changedId.impl()))
+            return true;
+    }
+    return false;
+}
+
 void IdChangeInvalidation::invalidateStyle(const AtomicString& changedId)
 {
     if (changedId.isEmpty())
@@ -51,11 +65,9 @@ void IdChangeInvalidation::invalidateStyle(const AtomicString& changedId)
 
     auto& ruleSets = m_element.styleResolver().ruleSets();
 
-    bool mayAffectStyle = ruleSets.features().idsInRules.contains(changedId.impl());
-
-    auto* shadowRoot = m_element.shadowRoot();
-    if (!mayAffectStyle && shadowRoot && mayBeAffectedByHostStyle(*shadowRoot, changedId))
-        mayAffectStyle = true;
+    bool mayAffectStyle = ruleSets.features().idsInRules.contains(changedId.impl())
+        || mayBeAffectedByHostRules(m_element, changedId)
+        || mayBeAffectedBySlottedRules(m_element, changedId);
 
     if (!mayAffectStyle)
         return;
index b518728..4571024 100644 (file)
@@ -288,7 +288,7 @@ bool SharingResolver::canShareStyleWithElement(const Context& context, const Sty
     if (candidateElement.matchesDefaultPseudoClass() != element.matchesDefaultPseudoClass())
         return false;
 
-    if (element.shadowRoot() && !element.shadowRoot()->styleScope().resolver().ruleSets().authorStyle().hostPseudoClassRules().isEmpty())
+    if (candidateElement.shadowRoot() && !candidateElement.shadowRoot()->styleScope().resolver().ruleSets().authorStyle().hostPseudoClassRules().isEmpty())
         return false;
 
 #if ENABLE(FULLSCREEN_API)