Source/WebCore:
authorantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 8 Dec 2016 14:05:27 +0000 (14:05 +0000)
committerantti@apple.com <antti@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 8 Dec 2016 14:05:27 +0000 (14:05 +0000)
::after and ::before don't work on :host
https://bugs.webkit.org/show_bug.cgi?id=164675
<rdar://problem/29231874>

Reviewed by Andreas Kling.

We didn't allow combining :host with any other selectors. Pseudo elements should be allowed.

Test: fast/shadow-dom/shadow-host-with-before-after.html

* css/ElementRuleCollector.cpp:
(WebCore::ElementRuleCollector::matchAuthorRules):

    Allow pseudo element matching.

(WebCore::ElementRuleCollector::matchHostPseudoClassRules):

    Use the normal collectMatchingRulesForList path instead of a direct call to SelectorChecker::matchHostPseudoClass.
    This path supports pseudo elements.

    Pass the information that we are matching :host rules with m_isMatchingHostPseudoClass bit
    similarly to how this is done with slots.

(WebCore::ElementRuleCollector::ruleMatches):
* css/ElementRuleCollector.h:
* css/RuleSet.cpp:

    Collect :host rules similarly to other rules.

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

    Rename didMoveToShadowHost->mayMatchHostPseudoClass for clarity.

(WebCore::SelectorChecker::match):

    Enable :host matching in the current context immediately if needed.

(WebCore::SelectorChecker::matchHostPseudoClass):

    The check for illegal combinations with :host is now done in checkOne().

(WebCore::localContextForParent):
(WebCore::SelectorChecker::checkOne):

    In context where :host matches nothing else can match, except pseudo elements.

* css/SelectorChecker.h:

LayoutTests:
:after and :before don't work on :host
https://bugs.webkit.org/show_bug.cgi?id=164675
<rdar://problem/29231874>

Reviewed by Andreas Kling.

* fast/shadow-dom/shadow-host-with-before-after-expected.html: Added.
* fast/shadow-dom/shadow-host-with-before-after.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/fast/shadow-dom/shadow-host-with-before-after-expected.html [new file with mode: 0644]
LayoutTests/fast/shadow-dom/shadow-host-with-before-after.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/css/ElementRuleCollector.cpp
Source/WebCore/css/ElementRuleCollector.h
Source/WebCore/css/RuleSet.cpp
Source/WebCore/css/SelectorChecker.cpp
Source/WebCore/css/SelectorChecker.h

index 5118e62..dee1d53 100644 (file)
@@ -1,3 +1,14 @@
+2016-12-08  Antti Koivisto  <antti@apple.com>
+
+        :after and :before don't work on :host
+        https://bugs.webkit.org/show_bug.cgi?id=164675
+        <rdar://problem/29231874>
+
+        Reviewed by Andreas Kling.
+
+        * fast/shadow-dom/shadow-host-with-before-after-expected.html: Added.
+        * fast/shadow-dom/shadow-host-with-before-after.html: Added.
+
 2016-12-08  Ryosuke Niwa  <rniwa@webkit.org>
 
         REGRESSION (r209490): [ios-simulator] LayoutTest fast/shadow-dom/media-shadow-manipulation.html is failing
diff --git a/LayoutTests/fast/shadow-dom/shadow-host-with-before-after-expected.html b/LayoutTests/fast/shadow-dom/shadow-host-with-before-after-expected.html
new file mode 100644 (file)
index 0000000..e704d24
--- /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/shadow-host-with-before-after.html b/LayoutTests/fast/shadow-dom/shadow-host-with-before-after.html
new file mode 100644 (file)
index 0000000..26fb4ac
--- /dev/null
@@ -0,0 +1,88 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+.test {
+    width: 100px;
+    height: 25px;
+    background: red;
+    color: red;
+}
+#host1, #host2  {
+    color: green;
+}
+#host3 div, #host4 div {
+    width: 50%;
+    height: 100%;
+    background: green;
+    display: inline-block;
+}
+</style>
+</head>
+<body>
+<p>Test passes if you see a single 100px by 100px green box below.</p> 
+<div id="host1" class="test"></div>
+<div id="host2" class="test"></div>
+<div id="host3" class="test"><div>text</div></div>
+<div id="host4" class="test"><div>text</div></div>
+<script>
+
+host1.attachShadow({mode: 'closed'}).innerHTML = `<style>
+    :host::before, :host::after {
+        background: green;
+        width: 50%;
+        height: 100%;
+        background: green;
+        display: inline-block;
+        content: "test";
+    }
+    </style>`;
+
+host2.attachShadow({mode: 'closed'}).innerHTML = `<style>
+    :host(.green)::before, :host(.green)::after {
+        background: green;
+        width: 50%;
+        height: 100%;
+        background: green;
+        display: inline-block;
+        content: "test";
+    }
+    </style>`;
+
+getComputedStyle(host2).backgroundColor;
+host2.classList.add('green');
+
+host3.attachShadow({mode: 'closed'}).innerHTML = `<style>
+    :host {
+        color: green !important;
+    }
+    :host::before {
+        background: green;
+        width: 50%;
+        height: 100%;
+        background: green;
+        display: inline-block;
+        content: "test";
+    }
+    </style><slot></slot>`;
+
+host4.attachShadow({mode: 'closed'}).innerHTML = `<style>
+    :host(.green) {
+        color: green !important;
+    }
+    :host(.green)::after {
+        background: green;
+        width: 50%;
+        height: 100%;
+        background: green;
+        display: inline-block;
+        content: "test";
+    }
+    </style><slot></slot>`;
+
+getComputedStyle(host4).backgroundColor;
+host4.classList.add('green');
+
+</script>
+</body>
+</html>
index e8d2e57..732c00c 100644 (file)
@@ -1,3 +1,54 @@
+2016-12-08  Antti Koivisto  <antti@apple.com>
+
+        ::after and ::before don't work on :host
+        https://bugs.webkit.org/show_bug.cgi?id=164675
+        <rdar://problem/29231874>
+
+        Reviewed by Andreas Kling.
+
+        We didn't allow combining :host with any other selectors. Pseudo elements should be allowed.
+
+        Test: fast/shadow-dom/shadow-host-with-before-after.html
+
+        * css/ElementRuleCollector.cpp:
+        (WebCore::ElementRuleCollector::matchAuthorRules):
+
+            Allow pseudo element matching.
+
+        (WebCore::ElementRuleCollector::matchHostPseudoClassRules):
+
+            Use the normal collectMatchingRulesForList path instead of a direct call to SelectorChecker::matchHostPseudoClass.
+            This path supports pseudo elements.
+
+            Pass the information that we are matching :host rules with m_isMatchingHostPseudoClass bit
+            similarly to how this is done with slots.
+
+        (WebCore::ElementRuleCollector::ruleMatches):
+        * css/ElementRuleCollector.h:
+        * css/RuleSet.cpp:
+
+            Collect :host rules similarly to other rules.
+
+        (WebCore::RuleSet::addRule):
+        * css/SelectorChecker.cpp:
+
+            Rename didMoveToShadowHost->mayMatchHostPseudoClass for clarity.
+
+        (WebCore::SelectorChecker::match):
+
+            Enable :host matching in the current context immediately if needed.
+
+        (WebCore::SelectorChecker::matchHostPseudoClass):
+
+            The check for illegal combinations with :host is now done in checkOne().
+
+        (WebCore::localContextForParent):
+        (WebCore::SelectorChecker::checkOne):
+
+            In context where :host matches nothing else can match, except pseudo elements.
+
+        * css/SelectorChecker.h:
+
 2016-12-08  Alex Christensen  <achristensen@webkit.org>
 
         Fix CMake build.
index f0fc6ee..fda5dc1 100644 (file)
@@ -214,7 +214,7 @@ void ElementRuleCollector::matchAuthorRules(bool includeEmptyRules)
     if (parent && parent->shadowRoot())
         matchSlottedPseudoElementRules(includeEmptyRules, ruleRange);
 
-    if (m_element.shadowRoot() && m_pseudoStyleRequest.pseudoId == NOPSEUDO)
+    if (m_element.shadowRoot())
         matchHostPseudoClassRules(includeEmptyRules, ruleRange);
 
     if (m_element.isInShadowTree())
@@ -244,18 +244,10 @@ void ElementRuleCollector::matchHostPseudoClassRules(bool includeEmptyRules, Sty
     if (shadowHostRules.isEmpty())
         return;
 
-    SelectorChecker::CheckingContext context(m_mode);
-    SelectorChecker selectorChecker(m_element.document());
+    SetForScope<bool> change(m_isMatchingHostPseudoClass, true);
 
-    for (auto& ruleData : shadowHostRules) {
-        if (ruleData.rule()->properties().isEmpty() && !includeEmptyRules)
-            continue;
-        auto& selector = *ruleData.selector();
-        unsigned specificity = 0;
-        if (!selectorChecker.matchHostPseudoClass(selector, m_element, context, specificity))
-            continue;
-        addMatchedRule(ruleData, specificity, Style::ScopeOrdinal::Shadow, ruleRange);
-    }
+    MatchRequest hostMatchRequest { nullptr, includeEmptyRules, Style::ScopeOrdinal::Shadow };
+    collectMatchingRulesForList(&shadowHostRules, hostMatchRequest, ruleRange);
 }
 
 void ElementRuleCollector::matchSlottedPseudoElementRules(bool includeEmptyRules, StyleResolver::RuleRange& ruleRange)
@@ -438,6 +430,7 @@ inline bool ElementRuleCollector::ruleMatches(const RuleData& ruleData, unsigned
     context.pseudoId = m_pseudoStyleRequest.pseudoId;
     context.scrollbar = m_pseudoStyleRequest.scrollbar;
     context.scrollbarPart = m_pseudoStyleRequest.scrollbarPart;
+    context.isMatchingHostPseudoClass = m_isMatchingHostPseudoClass;
 
     bool selectorMatches;
 #if ENABLE(CSS_SELECTOR_JIT)
index 3c381b5..3874c8b 100644 (file)
@@ -103,6 +103,7 @@ private:
     bool m_sameOriginOnly { false };
     SelectorChecker::Mode m_mode { SelectorChecker::Mode::ResolvingStyle };
     bool m_isMatchingSlottedPseudoElements { false };
+    bool m_isMatchingHostPseudoClass { false };
     Vector<std::unique_ptr<RuleSet::RuleDataVector>> m_keepAliveSlottedPseudoElementRules;
 
     Vector<MatchedRule, 64> m_matchedRules;
index 56442da..272f685 100644 (file)
@@ -205,6 +205,7 @@ void RuleSet::addRule(StyleRule* rule, unsigned selectorIndex, AddRuleFlags addR
     const CSSSelector* classSelector = nullptr;
     const CSSSelector* linkSelector = nullptr;
     const CSSSelector* focusSelector = nullptr;
+    const CSSSelector* hostPseudoClassSelector = nullptr;
     const CSSSelector* customPseudoElementSelector = nullptr;
     const CSSSelector* slottedPseudoElementSelector = nullptr;
 #if ENABLE(VIDEO_TRACK)
@@ -265,8 +266,8 @@ void RuleSet::addRule(StyleRule* rule, unsigned selectorIndex, AddRuleFlags addR
                 focusSelector = selector;
                 break;
             case CSSSelector::PseudoClassHost:
-                m_hostPseudoClassRules.append(ruleData);
-                return;
+                hostPseudoClassSelector = selector;
+                break;
             default:
                 break;
             }
@@ -308,6 +309,11 @@ void RuleSet::addRule(StyleRule* rule, unsigned selectorIndex, AddRuleFlags addR
         return;
     }
 
+    if (hostPseudoClassSelector) {
+        m_hostPseudoClassRules.append(ruleData);
+        return;
+    }
+
     if (idSelector) {
         addToRuleSet(idSelector->value().impl(), m_idRules, ruleData);
         return;
index e2db49d..e0056aa 100644 (file)
@@ -74,7 +74,7 @@ struct SelectorChecker::LocalContext {
     bool pseudoElementEffective { true };
     bool hasScrollbarPseudo { false };
     bool hasSelectionPseudo { false };
-    bool didMoveToShadowHost { false };
+    bool mayMatchHostPseudoClass { false };
 
 };
 
@@ -182,6 +182,12 @@ bool SelectorChecker::match(const CSSSelector& selector, const Element& element,
     specificity = 0;
 
     LocalContext context(selector, element, checkingContext.resolvingMode == SelectorChecker::Mode::QueryingRules ? VisitedMatchType::Disabled : VisitedMatchType::Enabled, checkingContext.pseudoId);
+
+    if (checkingContext.isMatchingHostPseudoClass) {
+        ASSERT(element.shadowRoot());
+        context.mayMatchHostPseudoClass = true;
+    }
+
     PseudoIdSet pseudoIdSet;
     MatchResult result = matchRecursively(checkingContext, context, pseudoIdSet, specificity);
     if (result.match != Match::SelectorMatches)
@@ -208,10 +214,6 @@ bool SelectorChecker::matchHostPseudoClass(const CSSSelector& selector, const El
 
     specificity = selector.simpleSelectorSpecificity();
 
-    // :host doesn't combine with any other selectors.
-    if (selector.tagHistory())
-        return false;
-    
     if (auto* selectorList = selector.selectorList()) {
         LocalContext context(*selectorList->first(), element, VisitedMatchType::Enabled, NOPSEUDO);
         context.inFunctionalPseudoClass = true;
@@ -245,7 +247,7 @@ static SelectorChecker::LocalContext localContextForParent(const SelectorChecker
 
     updatedContext.isMatchElement = false;
 
-    if (updatedContext.didMoveToShadowHost) {
+    if (updatedContext.mayMatchHostPseudoClass) {
         updatedContext.element = nullptr;
         return updatedContext;
     }
@@ -253,7 +255,7 @@ static SelectorChecker::LocalContext localContextForParent(const SelectorChecker
     // Move to the shadow host if matching :host and the parent is the shadow root.
     if (context.selector->match() == CSSSelector::PseudoClass && context.selector->pseudoClassType() == CSSSelector::PseudoClassHost && is<ShadowRoot>(context.element->parentNode())) {
         updatedContext.element = downcast<ShadowRoot>(*context.element->parentNode()).host();
-        updatedContext.didMoveToShadowHost = true;
+        updatedContext.mayMatchHostPseudoClass = true;
         return updatedContext;
     }
 
@@ -655,6 +657,14 @@ bool SelectorChecker::checkOne(CheckingContext& checkingContext, const LocalCont
 
     specificity = CSSSelector::addSpecificities(specificity, selector.simpleSelectorSpecificity());
 
+    if (context.mayMatchHostPseudoClass) {
+        // :host doesn't combine with anything except pseudo elements.
+        bool isHostPseudoClass = selector.match() == CSSSelector::PseudoClass && selector.pseudoClassType() == CSSSelector::PseudoClassHost;
+        bool isPseudoElement = selector.match() == CSSSelector::PseudoElement;
+        if (!isHostPseudoClass && !isPseudoElement)
+            return false;
+    }
+
     if (selector.match() == CSSSelector::Tag)
         return tagMatches(element, selector);
 
@@ -1039,7 +1049,7 @@ bool SelectorChecker::checkOne(CheckingContext& checkingContext, const LocalCont
             break;
         }
         case CSSSelector::PseudoClassHost: {
-            if (!context.didMoveToShadowHost)
+            if (!context.mayMatchHostPseudoClass)
                 return false;
             unsigned hostSpecificity;
             if (!matchHostPseudoClass(selector, element, checkingContext, hostSpecificity))
index cc3ba59..85e94ce 100644 (file)
@@ -84,6 +84,7 @@ public:
         RenderScrollbar* scrollbar { nullptr };
         ScrollbarPart scrollbarPart { NoPart };
         const ContainerNode* scope { nullptr };
+        bool isMatchingHostPseudoClass { false };
 
         // FIXME: It would be nicer to have a separate object for return values. This requires some more work in the selector compiler.
         Style::Relations styleRelations;