Updating class name of a shadow host does not update the style applied by descendants...
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 May 2017 12:31:49 +0000 (12:31 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 12 May 2017 12:31:49 +0000 (12:31 +0000)
https://bugs.webkit.org/show_bug.cgi?id=170762
<rdar://problem/31572668>

Reviewed by Ryosuke Niwa.

Source/WebCore:

We need to invalidate shadow tree style when host classes or attributes change if it may be
affected by host rules.

Test: fast/shadow-dom/css-scoping-host-class-and-attribute-mutation.html

* css/RuleSet.cpp:
(WebCore::isHostSelectorMatchingInShadowTree):
(WebCore::RuleSet::addRule):

    Check if we have :host selectors that affect shadow tree.

* css/RuleSet.h:
(WebCore::RuleSet::hasHostPseudoClassRulesMatchingInShadowTree):
* style/AttributeChangeInvalidation.cpp:
(WebCore::Style::mayBeAffectedByHostRules):
(WebCore::Style::AttributeChangeInvalidation::invalidateStyle):

    Invalidate the whole subtree if there is a class change that may affect shadow tree style.

* style/ClassChangeInvalidation.cpp:
(WebCore::Style::mayBeAffectedByHostRules):
(WebCore::Style::ClassChangeInvalidation::invalidateStyle):
* style/IdChangeInvalidation.cpp:
(WebCore::Style::mayBeAffectedByHostRules):
(WebCore::Style::IdChangeInvalidation::invalidateStyle):

    Same for classes and ids.
    This should be refactored at some point to reduce copy-code.

LayoutTests:

* fast/shadow-dom/css-scoping-host-class-and-attribute-mutation-expected.html: Added.
* fast/shadow-dom/css-scoping-host-class-and-attribute-mutation.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/shadow-dom/css-scoping-host-class-and-attribute-mutation-expected.html [new file with mode: 0644]
LayoutTests/fast/shadow-dom/css-scoping-host-class-and-attribute-mutation.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/RuleSet.cpp
Source/WebCore/css/RuleSet.h
Source/WebCore/style/AttributeChangeInvalidation.cpp
Source/WebCore/style/ClassChangeInvalidation.cpp
Source/WebCore/style/IdChangeInvalidation.cpp

index 22b701c..a382c57 100644 (file)
@@ -1,3 +1,14 @@
+2017-05-12  Antti Koivisto  <antti@apple.com>
+
+        Updating class name of a shadow host does not update the style applied by descendants of :host()
+        https://bugs.webkit.org/show_bug.cgi?id=170762
+        <rdar://problem/31572668>
+
+        Reviewed by Ryosuke Niwa.
+
+        * fast/shadow-dom/css-scoping-host-class-and-attribute-mutation-expected.html: Added.
+        * fast/shadow-dom/css-scoping-host-class-and-attribute-mutation.html: Added.
+
 2017-05-12  Claudio Saavedra  <csaavedra@igalia.com>
 
         [WPE] Unreviewed gardening.
diff --git a/LayoutTests/fast/shadow-dom/css-scoping-host-class-and-attribute-mutation-expected.html b/LayoutTests/fast/shadow-dom/css-scoping-host-class-and-attribute-mutation-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-host-class-and-attribute-mutation.html b/LayoutTests/fast/shadow-dom/css-scoping-host-class-and-attribute-mutation.html
new file mode 100644 (file)
index 0000000..61a9b21
--- /dev/null
@@ -0,0 +1,130 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<p>Test passes if you see a single 100px by 100px green box below.</p>
+
+<div id=host1></div>
+<div id=host2 class="red"></div>
+<div id=host3></div>
+<div id=host4></div>
+<div id=host5></div>
+
+<template id=shadow1>
+<style>
+:host(.green) div {
+    background-color: green;
+}
+div {
+    width: 100px;
+    height: 20px;
+    background-color: red;
+}
+</style>
+<nav><div></div></nav>
+</template>
+
+<template id=shadow2>
+<style>
+:host(.red) > div {
+    background-color: red;
+}
+div {
+    width: 100px;
+    height: 20px;
+    background-color: green;
+}
+</style>
+<div></div>
+</template>
+
+<template id=shadow3>
+<style>
+:host([color=green]) div {
+    background-color: green;
+}
+div {
+    width: 100px;
+    height: 20px;
+    background-color: red;
+}
+</style>
+<div></div>
+</template>
+
+<template id=shadow4>
+<style>
+:host(#green) div {
+    background-color: green;
+}
+div {
+    width: 100px;
+    height: 20px;
+    background-color: red;
+}
+</style>
+<div></div>
+</template>
+
+<template id=shadow5>
+<style>
+:host div.green {
+    background-color: green;
+}
+div {
+    width: 100px;
+    height: 20px;
+    background-color: red;
+}
+</style>
+<div></div>
+</template>
+
+<div id=log></div>
+
+<script>
+function checkColor(host, expected)
+{
+    const div = host.shadowRoot.querySelector("div");
+    const color = getComputedStyle(div).backgroundColor;
+    const width = getComputedStyle(div).width;
+    if (color != expected)
+        log.innerHTML += `FAIL: unexpected background color ${color}<br>`;
+    if (width != "100px")
+        log.innerHTML += `FAIL: unexpected width ${width}<br>`;
+}
+
+function test(hostselector, shadowselector, mutation) {
+    const host = document.querySelector(hostselector);
+    const shadowTemplate = document.querySelector(shadowselector);
+
+    host.attachShadow({ mode: 'open' });
+    host.shadowRoot.appendChild(shadowTemplate.content.cloneNode(true));
+
+    checkColor(host, "rgb(255, 0, 0)");
+
+    mutation(host);
+
+    checkColor(host, "rgb(0, 128, 0)");
+}
+
+test("#host1", "#shadow1", (host) => {
+    host.classList.add('green');
+});
+test("#host2", "#shadow2", (host) => {
+    host.classList.remove('red');
+ });
+test("#host3", "#shadow3", (host) => {
+    host.setAttribute('color','green');
+});
+test("#host4", "#shadow4", (host) => {
+   host.setAttribute('id','green');
+});
+test("#host5", "#shadow5", (host) => {
+   host.shadowRoot.querySelector('div').classList.add('green');
+});
+
+</script>
+</body>
+</html>
index 3152b07..f148e9b 100644 (file)
@@ -1,3 +1,40 @@
+2017-05-12  Antti Koivisto  <antti@apple.com>
+
+        Updating class name of a shadow host does not update the style applied by descendants of :host()
+        https://bugs.webkit.org/show_bug.cgi?id=170762
+        <rdar://problem/31572668>
+
+        Reviewed by Ryosuke Niwa.
+
+        We need to invalidate shadow tree style when host classes or attributes change if it may be
+        affected by host rules.
+
+        Test: fast/shadow-dom/css-scoping-host-class-and-attribute-mutation.html
+
+        * css/RuleSet.cpp:
+        (WebCore::isHostSelectorMatchingInShadowTree):
+        (WebCore::RuleSet::addRule):
+
+            Check if we have :host selectors that affect shadow tree.
+
+        * css/RuleSet.h:
+        (WebCore::RuleSet::hasHostPseudoClassRulesMatchingInShadowTree):
+        * style/AttributeChangeInvalidation.cpp:
+        (WebCore::Style::mayBeAffectedByHostRules):
+        (WebCore::Style::AttributeChangeInvalidation::invalidateStyle):
+
+            Invalidate the whole subtree if there is a class change that may affect shadow tree style.
+
+        * style/ClassChangeInvalidation.cpp:
+        (WebCore::Style::mayBeAffectedByHostRules):
+        (WebCore::Style::ClassChangeInvalidation::invalidateStyle):
+        * style/IdChangeInvalidation.cpp:
+        (WebCore::Style::mayBeAffectedByHostRules):
+        (WebCore::Style::IdChangeInvalidation::invalidateStyle):
+
+            Same for classes and ids.
+            This should be refactored at some point to reduce copy-code.
+
 2017-05-12  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         [GTK] ASSERTION FAILED: !m_flushingLayers
index 9e6b7e9..6d927d3 100644 (file)
@@ -194,6 +194,20 @@ static unsigned rulesCountForName(const RuleSet::AtomRuleMap& map, const AtomicS
     return 0;
 }
 
+static bool isHostSelectorMatchingInShadowTree(const CSSSelector& startSelector)
+{
+    auto* leftmostSelector = &startSelector;
+    bool hasDescendantOrChildRelation = false;
+    while (auto* previous = leftmostSelector->tagHistory()) {
+        hasDescendantOrChildRelation = leftmostSelector->hasDescendantOrChildRelation();
+        leftmostSelector = previous;
+    }
+    if (!hasDescendantOrChildRelation)
+        return false;
+
+    return leftmostSelector->match() == CSSSelector::PseudoClass && leftmostSelector->pseudoClassType() == CSSSelector::PseudoClassHost;
+}
+
 void RuleSet::addRule(StyleRule* rule, unsigned selectorIndex, AddRuleFlags addRuleFlags)
 {
     RuleData ruleData(rule, selectorIndex, m_ruleCount++, addRuleFlags);
@@ -309,6 +323,9 @@ void RuleSet::addRule(StyleRule* rule, unsigned selectorIndex, AddRuleFlags addR
         return;
     }
 
+    if (!m_hasHostPseudoClassRulesMatchingInShadowTree)
+        m_hasHostPseudoClassRulesMatchingInShadowTree = isHostSelectorMatchingInShadowTree(*ruleData.selector());
+
     if (hostPseudoClassSelector) {
         m_hostPseudoClassRules.append(ruleData);
         return;
index 6263cc7..df1472b 100644 (file)
@@ -192,6 +192,7 @@ public:
     unsigned ruleCount() const { return m_ruleCount; }
 
     bool hasShadowPseudoElementRules() const;
+    bool hasHostPseudoClassRulesMatchingInShadowTree() const { return m_hasHostPseudoClassRulesMatchingInShadowTree; }
 
 private:
     void addChildRules(const Vector<RefPtr<StyleRuleBase>>&, const MediaQueryEvaluator& medium, StyleResolver*, bool hasDocumentSecurityOrigin, bool isInitiatingElementInUserAgentShadowTree, AddRuleFlags);
@@ -211,6 +212,7 @@ private:
     RuleDataVector m_universalRules;
     Vector<StyleRulePage*> m_pageRules;
     unsigned m_ruleCount { 0 };
+    bool m_hasHostPseudoClassRulesMatchingInShadowTree { false };
     bool m_autoShrinkToFitEnabled { true };
     RuleFeatureSet m_features;
     Vector<RuleSetSelectorPair> m_regionSelectorsAndRuleSets;
index 35dda59..a090a2a 100644 (file)
@@ -42,16 +42,23 @@ static bool mayBeAffectedByAttributeChange(DocumentRuleSets& ruleSets, bool isHT
     return nameSet.contains(attributeName.localName());
 }
 
-static bool mayBeAffectedByHostRules(const Element& element, const QualifiedName& attributeName)
+static bool mayBeAffectedByHostRules(const Element& element, const QualifiedName& attributeName, bool& mayAffectShadowTree)
 {
+    // FIXME: More of this code should be shared between Class/Attribute/IdInvalidation.
     auto* shadowRoot = element.shadowRoot();
     if (!shadowRoot)
         return false;
     auto& shadowRuleSets = shadowRoot->styleScope().resolver().ruleSets();
-    if (shadowRuleSets.authorStyle().hostPseudoClassRules().isEmpty())
+    auto& authorStyle = shadowRuleSets.authorStyle();
+    if (authorStyle.hostPseudoClassRules().isEmpty() && !authorStyle.hasHostPseudoClassRulesMatchingInShadowTree())
         return false;
 
-    return mayBeAffectedByAttributeChange(shadowRuleSets, element.isHTMLElement(), attributeName);
+    if (!mayBeAffectedByAttributeChange(shadowRuleSets, element.isHTMLElement(), attributeName))
+        return false;
+
+    if (authorStyle.hasHostPseudoClassRulesMatchingInShadowTree())
+        mayAffectShadowTree = true;
+    return true;
 }
 
 static bool mayBeAffectedBySlottedRules(const Element& element, const QualifiedName& attributeName)
@@ -73,9 +80,10 @@ void AttributeChangeInvalidation::invalidateStyle(const QualifiedName& attribute
 
     auto& ruleSets = m_element.styleResolver().ruleSets();
     bool isHTML = m_element.isHTMLElement();
+    bool mayAffectShadowTree = false;
 
     bool mayAffectStyle = mayBeAffectedByAttributeChange(ruleSets, isHTML, attributeName)
-        || mayBeAffectedByHostRules(m_element, attributeName)
+        || mayBeAffectedByHostRules(m_element, attributeName, mayAffectShadowTree)
         || mayBeAffectedBySlottedRules(m_element, attributeName);
 
     if (!mayAffectStyle)
@@ -86,7 +94,10 @@ void AttributeChangeInvalidation::invalidateStyle(const QualifiedName& attribute
         return;
     }
 
-    if (m_element.shadowRoot() && ruleSets.authorStyle().hasShadowPseudoElementRules()) {
+    if (m_element.shadowRoot() && ruleSets.authorStyle().hasShadowPseudoElementRules())
+        mayAffectShadowTree = true;
+
+    if (mayAffectShadowTree) {
         m_element.invalidateStyleForSubtree();
         return;
     }
index d6660fb..961c0aa 100644 (file)
@@ -86,14 +86,22 @@ static ClassChangeVector computeClassChange(const SpaceSplitString& oldClasses,
     return changedClasses;
 }
 
-static bool mayBeAffectedByHostRules(ShadowRoot* shadowRoot, AtomicStringImpl* changedClass)
+static bool mayBeAffectedByHostRules(ShadowRoot* shadowRoot, AtomicStringImpl* changedClass, bool& mayAffectShadowTree)
 {
+    // FIXME: More of this code should be shared between Class/Attribute/IdInvalidation.
     if (!shadowRoot)
         return false;
     auto& shadowRuleSets = shadowRoot->styleScope().resolver().ruleSets();
-    if (shadowRuleSets.authorStyle().hostPseudoClassRules().isEmpty())
+    auto& authorStyle = shadowRuleSets.authorStyle();
+    if (authorStyle.hostPseudoClassRules().isEmpty() && !authorStyle.hasHostPseudoClassRulesMatchingInShadowTree())
         return false;
-    return shadowRuleSets.features().classesInRules.contains(changedClass);
+
+    if (!shadowRuleSets.features().classesInRules.contains(changedClass))
+        return false;
+
+    if (authorStyle.hasHostPseudoClassRulesMatchingInShadowTree())
+        mayAffectShadowTree = true;
+    return true;
 }
 
 static bool mayBeAffectedBySlottedRules(const Vector<ShadowRoot*>& assignedShadowRoots, AtomicStringImpl* changedClass)
@@ -111,6 +119,7 @@ static bool mayBeAffectedBySlottedRules(const Vector<ShadowRoot*>& assignedShado
 void ClassChangeInvalidation::invalidateStyle(const SpaceSplitString& oldClasses, const SpaceSplitString& newClasses)
 {
     auto changedClasses = computeClassChange(oldClasses, newClasses);
+    bool mayAffectShadowTree = false;
 
     auto& ruleSets = m_element.styleResolver().ruleSets();
     auto* shadowRoot = m_element.shadowRoot();
@@ -119,7 +128,7 @@ void ClassChangeInvalidation::invalidateStyle(const SpaceSplitString& oldClasses
     ClassChangeVector changedClassesAffectingStyle;
     for (auto* changedClass : changedClasses) {
         bool mayAffectStyle = ruleSets.features().classesInRules.contains(changedClass)
-            || mayBeAffectedByHostRules(shadowRoot, changedClass)
+            || mayBeAffectedByHostRules(shadowRoot, changedClass, mayAffectShadowTree)
             || mayBeAffectedBySlottedRules(assignedShadowRoots, changedClass);
         if (mayAffectStyle)
             changedClassesAffectingStyle.append(changedClass);
@@ -128,7 +137,11 @@ void ClassChangeInvalidation::invalidateStyle(const SpaceSplitString& oldClasses
     if (changedClassesAffectingStyle.isEmpty())
         return;
 
-    if (shadowRoot && ruleSets.authorStyle().hasShadowPseudoElementRules()) {
+    if (shadowRoot && ruleSets.authorStyle().hasShadowPseudoElementRules())
+        mayAffectShadowTree = true;
+
+    if (mayAffectShadowTree) {
+        // FIXME: We should do fine-grained invalidation for shadow tree.
         m_element.invalidateStyleForSubtree();
         return;
     }
index a0d99fe..826f25e 100644 (file)
 namespace WebCore {
 namespace Style {
 
-static bool mayBeAffectedByHostRules(const Element& element, const AtomicString& changedId)
+static bool mayBeAffectedByHostRules(const Element& element, const AtomicString& changedId, bool& mayAffectShadowTree)
 {
     auto* shadowRoot = element.shadowRoot();
     if (!shadowRoot)
         return false;
     auto& shadowRuleSets = shadowRoot->styleScope().resolver().ruleSets();
-    if (shadowRuleSets.authorStyle().hostPseudoClassRules().isEmpty())
+    auto& authorStyle = shadowRuleSets.authorStyle();
+    if (authorStyle.hostPseudoClassRules().isEmpty() && !authorStyle.hasHostPseudoClassRulesMatchingInShadowTree())
         return false;
-    return shadowRuleSets.features().idsInRules.contains(changedId);
+
+    if (!shadowRuleSets.features().idsInRules.contains(changedId))
+        return false;
+
+    if (authorStyle.hasHostPseudoClassRulesMatchingInShadowTree())
+        mayAffectShadowTree = true;
+    return true;
 }
 
 static bool mayBeAffectedBySlottedRules(const Element& element, const AtomicString& changedId)
@@ -64,15 +71,19 @@ void IdChangeInvalidation::invalidateStyle(const AtomicString& changedId)
         return;
 
     auto& ruleSets = m_element.styleResolver().ruleSets();
+    bool mayAffectShadowTree = false;
 
     bool mayAffectStyle = ruleSets.features().idsInRules.contains(changedId)
-        || mayBeAffectedByHostRules(m_element, changedId)
+        || mayBeAffectedByHostRules(m_element, changedId, mayAffectShadowTree)
         || mayBeAffectedBySlottedRules(m_element, changedId);
 
     if (!mayAffectStyle)
         return;
 
-    if (m_element.shadowRoot() && ruleSets.authorStyle().hasShadowPseudoElementRules()) {
+    if (m_element.shadowRoot() && ruleSets.authorStyle().hasShadowPseudoElementRules())
+        mayAffectShadowTree = true;
+
+    if (mayAffectShadowTree) {
         m_element.invalidateStyleForSubtree();
         return;
     }