[CSS Parser] Fix compound selector parsing.
authorhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Oct 2016 15:07:19 +0000 (15:07 +0000)
committerhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 19 Oct 2016 15:07:19 +0000 (15:07 +0000)
https://bugs.webkit.org/show_bug.cgi?id=163649

Reviewed by Darin Adler.

The new CSS parser is failing to handle compound selectors. The code has an assumption that the
first value in the RelationType enum is SubSelector. This patch changes the enum to have the same
name used in Blink, RelationType, and to make the ordering be exactly the same.

* css/CSSSelector.h:
(WebCore::CSSSelector::relation):
(WebCore::CSSSelector::setRelation):
* css/SelectorChecker.cpp:
(WebCore::SelectorChecker::matchRecursively):
(WebCore::canMatchHoverOrActiveInQuirksMode):
(WebCore::SelectorChecker::determineLinkMatchType):
* css/SelectorFilter.cpp:
(WebCore::SelectorFilter::collectIdentifierHashes):
* css/parser/CSSParserValues.cpp:
(WebCore::CSSParserSelector::insertTagHistory):
(WebCore::CSSParserSelector::appendTagHistory):
* css/parser/CSSParserValues.h:
(WebCore::CSSParserSelector::setRelation):
* css/parser/CSSSelectorParser.cpp:
(WebCore::CSSSelectorParser::consumeComplexSelector):
(WebCore::CSSSelectorParser::consumeCombinator):
* css/parser/CSSSelectorParser.h:
* cssjit/SelectorCompiler.cpp:
(WebCore::SelectorCompiler::fragmentRelationForSelectorRelation):
(WebCore::SelectorCompiler::constructFragmentsInternal):

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

14 files changed:
Source/WebCore/ChangeLog
Source/WebCore/css/CSSSelector.cpp
Source/WebCore/css/CSSSelector.h
Source/WebCore/css/CSSSelectorList.cpp
Source/WebCore/css/RuleSet.cpp
Source/WebCore/css/SelectorChecker.cpp
Source/WebCore/css/SelectorFilter.cpp
Source/WebCore/css/parser/CSSParser.cpp
Source/WebCore/css/parser/CSSParserValues.cpp
Source/WebCore/css/parser/CSSParserValues.h
Source/WebCore/css/parser/CSSSelectorParser.cpp
Source/WebCore/css/parser/CSSSelectorParser.h
Source/WebCore/cssjit/SelectorCompiler.cpp
Source/WebCore/dom/SelectorQuery.cpp

index cb5c1e7..85766f0 100644 (file)
@@ -1,3 +1,36 @@
+2016-10-18  Dave Hyatt  <hyatt@apple.com>
+
+        [CSS Parser] Fix compound selector parsing.
+        https://bugs.webkit.org/show_bug.cgi?id=163649
+
+        Reviewed by Darin Adler.
+
+        The new CSS parser is failing to handle compound selectors. The code has an assumption that the
+        first value in the RelationType enum is SubSelector. This patch changes the enum to have the same
+        name used in Blink, RelationType, and to make the ordering be exactly the same.
+
+        * css/CSSSelector.h:
+        (WebCore::CSSSelector::relation):
+        (WebCore::CSSSelector::setRelation):
+        * css/SelectorChecker.cpp:
+        (WebCore::SelectorChecker::matchRecursively):
+        (WebCore::canMatchHoverOrActiveInQuirksMode):
+        (WebCore::SelectorChecker::determineLinkMatchType):
+        * css/SelectorFilter.cpp:
+        (WebCore::SelectorFilter::collectIdentifierHashes):
+        * css/parser/CSSParserValues.cpp:
+        (WebCore::CSSParserSelector::insertTagHistory):
+        (WebCore::CSSParserSelector::appendTagHistory):
+        * css/parser/CSSParserValues.h:
+        (WebCore::CSSParserSelector::setRelation):
+        * css/parser/CSSSelectorParser.cpp:
+        (WebCore::CSSSelectorParser::consumeComplexSelector):
+        (WebCore::CSSSelectorParser::consumeCombinator):
+        * css/parser/CSSSelectorParser.h:
+        * cssjit/SelectorCompiler.cpp:
+        (WebCore::SelectorCompiler::fragmentRelationForSelectorRelation):
+        (WebCore::SelectorCompiler::constructFragmentsInternal):
+
 2016-10-19  Javier Fernandez  <jfernandez@igalia.com>
 
         Web Inspector: Debugger buttons positioned incorrectly, align-content default value is unexpected
index 15557d0..5d5847b 100644 (file)
@@ -46,6 +46,7 @@ struct SameSizeAsCSSSelector {
     void* unionPointer;
 };
 
+static_assert(CSSSelector::RelationType::Subselector == 0, "Subselector must be 0 for consumeCombinator.");
 static_assert(sizeof(CSSSelector) == sizeof(SameSizeAsCSSSelector), "CSSSelector should remain small.");
 
 CSSSelector::CSSSelector(const QualifiedName& tagQName, bool tagIsForNamespaceRule)
@@ -720,7 +721,7 @@ String CSSSelector::selectorText(const String& rightSide) const
             }
         }
 
-        if (cs->relation() != CSSSelector::SubSelector || !cs->tagHistory())
+        if (cs->relation() != CSSSelector::Subselector || !cs->tagHistory())
             break;
         cs = cs->tagHistory();
     }
@@ -741,7 +742,7 @@ String CSSSelector::selectorText(const String& rightSide) const
             return tagHistory->selectorText(" + " + str.toString() + rightSide);
         case CSSSelector::IndirectAdjacent:
             return tagHistory->selectorText(" ~ " + str.toString() + rightSide);
-        case CSSSelector::SubSelector:
+        case CSSSelector::Subselector:
             ASSERT_NOT_REACHED();
 #if ASSERT_DISABLED
             FALLTHROUGH;
index e81ba69..36c6743 100644 (file)
@@ -81,12 +81,12 @@ namespace WebCore {
             PagePseudoClass
         };
 
-        enum Relation {
-            Descendant = 0,
+        enum RelationType {
+            Subselector,
+            Descendant,
             Child,
             DirectAdjacent,
             IndirectAdjacent,
-            SubSelector,
             ShadowDescendant, // FIXME-NEWPARSER: Remove this in favor of the new shadow values below.
             ShadowPseudo, // Special case of shadow DOM pseudo elements / shadow pseudo element
             ShadowDeep, // /deep/ combinator
@@ -295,8 +295,8 @@ namespace WebCore {
         bool isSiblingSelector() const;
         bool isAttributeSelector() const;
 
-        Relation relation() const { return static_cast<Relation>(m_relation); }
-        void setRelation(Relation relation)
+        RelationType relation() const { return static_cast<RelationType>(m_relation); }
+        void setRelation(RelationType relation)
         {
             m_relation = relation;
             ASSERT(m_relation == relation);
@@ -326,7 +326,7 @@ namespace WebCore {
         void setForPage() { m_isForPage = true; }
 
     private:
-        unsigned m_relation              : 3; // enum Relation.
+        unsigned m_relation              : 3; // enum RelationType.
         mutable unsigned m_match         : 4; // enum Match.
         mutable unsigned m_pseudoType    : 8; // PseudoType.
         mutable unsigned m_parsedNth     : 1; // Used for :nth-*.
index 53ae69b..bd13a6a 100644 (file)
@@ -131,9 +131,9 @@ String CSSSelectorList::selectorsText() const
 
 void CSSSelectorList::buildSelectorsText(StringBuilder& stringBuilder) const
 {
-    const CSSSelector* firstSubSelector = first();
-    for (const CSSSelector* subSelector = firstSubSelector; subSelector; subSelector = CSSSelectorList::next(subSelector)) {
-        if (subSelector != firstSubSelector)
+    const CSSSelector* firstSubselector = first();
+    for (const CSSSelector* subSelector = firstSubselector; subSelector; subSelector = CSSSelectorList::next(subSelector)) {
+        if (subSelector != firstSubselector)
             stringBuilder.appendLiteral(", ");
         stringBuilder.append(subSelector->selectorText());
     }
index cca2bc3..1343527 100644 (file)
@@ -122,7 +122,7 @@ static bool containsUncommonAttributeSelector(const CSSSelector& rootSelector, b
             }
         }
 
-        if (selector->relation() != CSSSelector::SubSelector)
+        if (selector->relation() != CSSSelector::Subselector)
             matchesRightmostElement = false;
 
         selector = selector->tagHistory();
@@ -268,7 +268,7 @@ void RuleSet::addRule(StyleRule* rule, unsigned selectorIndex, AddRuleFlags addR
             m_slottedPseudoElementRules.append(ruleData);
             return;
         }
-        if (selector->relation() != CSSSelector::SubSelector)
+        if (selector->relation() != CSSSelector::Subselector)
             break;
         selector = selector->tagHistory();
     } while (selector);
index 44c7db4..5951ba6 100644 (file)
@@ -311,7 +311,7 @@ SelectorChecker::MatchResult SelectorChecker::matchRecursively(CheckingContext&
     }
 
     // The rest of the selectors has to match
-    CSSSelector::Relation relation = context.selector->relation();
+    auto relation = context.selector->relation();
 
     // Prepare next selector
     const CSSSelector* leftSelector = context.selector->tagHistory();
@@ -321,7 +321,7 @@ SelectorChecker::MatchResult SelectorChecker::matchRecursively(CheckingContext&
     LocalContext nextContext(context);
     nextContext.selector = leftSelector;
 
-    if (relation != CSSSelector::SubSelector) {
+    if (relation != CSSSelector::Subselector) {
         // Bail-out if this selector is irrelevant for the pseudoId
         if (context.pseudoId != NOPSEUDO && !dynamicPseudoIdSet.has(context.pseudoId))
             return MatchResult::fails(Match::SelectorFailsCompletely);
@@ -416,7 +416,7 @@ SelectorChecker::MatchResult SelectorChecker::matchRecursively(CheckingContext&
         };
         return MatchResult::fails(Match::SelectorFailsAllSiblings);
 
-    case CSSSelector::SubSelector:
+    case CSSSelector::Subselector:
         {
             // a selector is invalid if something follows a pseudo-element
             // We make an exception for scrollbar pseudo elements and allow a set of pseudo classes (but nothing else)
@@ -634,11 +634,11 @@ static bool canMatchHoverOrActiveInQuirksMode(const SelectorChecker::LocalContex
             break;
         }
 
-        CSSSelector::Relation relation = selector->relation();
+        auto relation = selector->relation();
         if (relation == CSSSelector::ShadowDescendant)
             return true;
 
-        if (relation != CSSSelector::SubSelector)
+        if (relation != CSSSelector::Subselector)
             return false;
     }
     return false;
@@ -1201,8 +1201,8 @@ unsigned SelectorChecker::determineLinkMatchType(const CSSSelector* selector)
                 break;
             }
         }
-        CSSSelector::Relation relation = selector->relation();
-        if (relation == CSSSelector::SubSelector)
+        auto relation = selector->relation();
+        if (relation == CSSSelector::Subselector)
             continue;
         if (relation != CSSSelector::Descendant && relation != CSSSelector::Child)
             return linkMatchType;
index 6bd1fc7..ae62236 100644 (file)
@@ -122,14 +122,14 @@ void SelectorFilter::collectIdentifierHashes(const CSSSelector* selector, unsign
 {
     unsigned* hash = identifierHashes;
     unsigned* end = identifierHashes + maximumIdentifierCount;
-    CSSSelector::Relation relation = selector->relation();
+    auto relation = selector->relation();
 
     // Skip the topmost selector. It is handled quickly by the rule hashes.
     bool skipOverSubselectors = true;
     for (selector = selector->tagHistory(); selector; selector = selector->tagHistory()) {
         // Only collect identifiers that match ancestors.
         switch (relation) {
-        case CSSSelector::SubSelector:
+        case CSSSelector::Subselector:
             if (!skipOverSubselectors)
                 collectDescendantSelectorIdentifierHashes(selector, hash);
             break;
index f857b8b..94c5e9e 100644 (file)
@@ -12981,10 +12981,10 @@ std::unique_ptr<CSSParserSelector> CSSParser::rewriteSpecifiers(std::unique_ptr<
     }
     if (specifiers->isCustomPseudoElement()) {
         // Specifiers for unknown pseudo element go right behind it in the chain.
-        specifiers->insertTagHistory(CSSSelector::SubSelector, WTFMove(newSpecifier), CSSSelector::ShadowDescendant);
+        specifiers->insertTagHistory(CSSSelector::Subselector, WTFMove(newSpecifier), CSSSelector::ShadowDescendant);
         return specifiers;
     }
-    specifiers->appendTagHistory(CSSSelector::SubSelector, WTFMove(newSpecifier));
+    specifiers->appendTagHistory(CSSSelector::Subselector, WTFMove(newSpecifier));
     return specifiers;
 }
 
index a3afc58..82af72e 100644 (file)
@@ -447,7 +447,7 @@ bool CSSParserSelector::matchesPseudoElement() const
     return m_selector->matchesPseudoElement() || selectorListMatchesPseudoElement(m_selector->selectorList());
 }
 
-void CSSParserSelector::insertTagHistory(CSSSelector::Relation before, std::unique_ptr<CSSParserSelector> selector, CSSSelector::Relation after)
+void CSSParserSelector::insertTagHistory(CSSSelector::RelationType before, std::unique_ptr<CSSParserSelector> selector, CSSSelector::RelationType after)
 {
     if (m_tagHistory)
         selector->setTagHistory(WTFMove(m_tagHistory));
@@ -456,7 +456,7 @@ void CSSParserSelector::insertTagHistory(CSSSelector::Relation before, std::uniq
     m_tagHistory = WTFMove(selector);
 }
 
-void CSSParserSelector::appendTagHistory(CSSSelector::Relation relation, std::unique_ptr<CSSParserSelector> selector)
+void CSSParserSelector::appendTagHistory(CSSSelector::RelationType relation, std::unique_ptr<CSSParserSelector> selector)
 {
     CSSParserSelector* end = this;
     while (end->tagHistory())
@@ -472,7 +472,7 @@ void CSSParserSelector::appendTagHistory(CSSParserSelectorCombinator relation, s
     while (end->tagHistory())
         end = end->tagHistory();
 
-    CSSSelector::Relation selectorRelation;
+    CSSSelector::RelationType selectorRelation;
     switch (relation) {
     case CSSParserSelectorCombinator::Child:
         selectorRelation = CSSSelector::Child;
@@ -519,12 +519,12 @@ void CSSParserSelector::prependTagSelector(const QualifiedName& tagQName, bool t
     m_tagHistory = WTFMove(second);
 
     m_selector = std::make_unique<CSSSelector>(tagQName, tagIsForNamespaceRule);
-    m_selector->setRelation(CSSSelector::SubSelector);
+    m_selector->setRelation(CSSSelector::Subselector);
 }
 
 std::unique_ptr<CSSParserSelector> CSSParserSelector::releaseTagHistory()
 {
-    setRelation(CSSSelector::SubSelector);
+    setRelation(CSSSelector::Subselector);
     return WTFMove(m_tagHistory);
 }
 
index 2125917..6869c3b 100644 (file)
@@ -226,7 +226,7 @@ public:
     void setArgument(const AtomicString& value) { m_selector->setArgument(value); }
     void setAttributeValueMatchingIsCaseInsensitive(bool isCaseInsensitive) { m_selector->setAttributeValueMatchingIsCaseInsensitive(isCaseInsensitive); }
     void setMatch(CSSSelector::Match value) { m_selector->setMatch(value); }
-    void setRelation(CSSSelector::Relation value) { m_selector->setRelation(value); }
+    void setRelation(CSSSelector::RelationType value) { m_selector->setRelation(value); }
     void setForPage() { m_selector->setForPage(); }
 
     CSSSelector::Match match() const { return m_selector->match(); }
@@ -261,8 +261,8 @@ public:
     CSSParserSelector* tagHistory() const { return m_tagHistory.get(); }
     void setTagHistory(std::unique_ptr<CSSParserSelector> selector) { m_tagHistory = WTFMove(selector); }
     void clearTagHistory() { m_tagHistory.reset(); }
-    void insertTagHistory(CSSSelector::Relation before, std::unique_ptr<CSSParserSelector>, CSSSelector::Relation after);
-    void appendTagHistory(CSSSelector::Relation, std::unique_ptr<CSSParserSelector>);
+    void insertTagHistory(CSSSelector::RelationType before, std::unique_ptr<CSSParserSelector>, CSSSelector::RelationType after);
+    void appendTagHistory(CSSSelector::RelationType, std::unique_ptr<CSSParserSelector>);
     void appendTagHistory(CSSParserSelectorCombinator, std::unique_ptr<CSSParserSelector>);
     void prependTagSelector(const QualifiedName&, bool tagIsForNamespaceRule = false);
     std::unique_ptr<CSSParserSelector> releaseTagHistory();
index 2485f4a..021f521 100644 (file)
@@ -132,13 +132,12 @@ std::unique_ptr<CSSParserSelector> CSSSelectorParser::consumeComplexSelector(CSS
     if (!selector)
         return nullptr;
 
-
     unsigned previousCompoundFlags = 0;
 
     for (CSSParserSelector* simple = selector.get(); simple && !previousCompoundFlags; simple = simple->tagHistory())
         previousCompoundFlags |= extractCompoundFlags(*simple, m_context.mode);
 
-    while (CSSSelector::Relation combinator = consumeCombinator(range)) {
+    while (auto combinator = consumeCombinator(range)) {
         std::unique_ptr<CSSParserSelector> nextSelector = consumeCompoundSelector(range);
         if (!nextSelector)
             return combinator == CSSSelector::Descendant ? WTFMove(selector) : nullptr;
@@ -533,9 +532,9 @@ std::unique_ptr<CSSParserSelector> CSSSelectorParser::consumePseudo(CSSParserTok
     return nullptr;
 }
 
-CSSSelector::Relation CSSSelectorParser::consumeCombinator(CSSParserTokenRange& range)
+CSSSelector::RelationType CSSSelectorParser::consumeCombinator(CSSParserTokenRange& range)
 {
-    CSSSelector::Relation fallbackResult = CSSSelector::SubSelector;
+    auto fallbackResult = CSSSelector::Subselector;
     while (range.peek().type() == WhitespaceToken) {
         range.consume();
         fallbackResult = CSSSelector::Descendant;
@@ -732,7 +731,7 @@ void CSSSelectorParser::prependTypeSelectorIfNeeded(const AtomicString& namespac
 
 std::unique_ptr<CSSParserSelector> CSSSelectorParser::addSimpleSelectorToCompound(std::unique_ptr<CSSParserSelector> compoundSelector, std::unique_ptr<CSSParserSelector> simpleSelector)
 {
-    compoundSelector->appendTagHistory(CSSSelector::SubSelector, WTFMove(simpleSelector));
+    compoundSelector->appendTagHistory(CSSSelector::Subselector, WTFMove(simpleSelector));
     return compoundSelector;
 }
 
@@ -743,7 +742,7 @@ std::unique_ptr<CSSParserSelector> CSSSelectorParser::splitCompoundAtImplicitSha
     // from left-to-right.
     //
     // ".a.b > div#id" is stored in a tagHistory as [div, #id, .a, .b], each element in the
-    // list stored with an associated relation (combinator or SubSelector).
+    // list stored with an associated relation (combinator or Subselector).
     //
     // ::cue, ::shadow, and custom pseudo elements have an implicit ShadowPseudo combinator
     // to their left, which really makes for a new compound selector, yet it's consumed by
index 2182740..9cacd6d 100644 (file)
@@ -68,7 +68,7 @@ private:
     std::unique_ptr<CSSParserSelector> consumePseudo(CSSParserTokenRange&);
     std::unique_ptr<CSSParserSelector> consumeAttribute(CSSParserTokenRange&);
 
-    CSSSelector::Relation consumeCombinator(CSSParserTokenRange&);
+    CSSSelector::RelationType consumeCombinator(CSSParserTokenRange&);
     CSSSelector::Match consumeAttributeMatch(CSSParserTokenRange&);
     CSSSelector::AttributeMatchType consumeAttributeFlags(CSSParserTokenRange&);
 
index f0eefa8..c786ef9 100644 (file)
@@ -398,7 +398,7 @@ SelectorCompilationStatus compileSelector(const CSSSelector* lastSelector, JSC::
     return codeGenerator.compile(vm, codeRef);
 }
 
-static inline FragmentRelation fragmentRelationForSelectorRelation(CSSSelector::Relation relation)
+static inline FragmentRelation fragmentRelationForSelectorRelation(CSSSelector::RelationType relation)
 {
     switch (relation) {
     case CSSSelector::Descendant:
@@ -409,7 +409,7 @@ static inline FragmentRelation fragmentRelationForSelectorRelation(CSSSelector::
         return FragmentRelation::DirectAdjacent;
     case CSSSelector::IndirectAdjacent:
         return FragmentRelation::IndirectAdjacent;
-    case CSSSelector::SubSelector:
+    case CSSSelector::Subselector:
     case CSSSelector::ShadowDescendant:
     case CSSSelector::ShadowPseudo:
     case CSSSelector::ShadowDeep:
@@ -998,8 +998,8 @@ static FunctionType constructFragmentsInternal(const CSSSelector* rootSelector,
             return FunctionType::CannotMatchAnything;
         }
 
-        CSSSelector::Relation relation = selector->relation();
-        if (relation == CSSSelector::SubSelector)
+        auto relation = selector->relation();
+        if (relation == CSSSelector::Subselector)
             continue;
 
         if (relation == CSSSelector::ShadowDescendant && !selector->isLastInTagHistory())
index 557dbe5..859c6b2 100644 (file)
@@ -70,7 +70,7 @@ static IdMatchingType findIdMatchingType(const CSSSelector& firstSelector)
                 return IdMatchingType::Rightmost;
             return IdMatchingType::Filter;
         }
-        if (selector->relation() != CSSSelector::SubSelector)
+        if (selector->relation() != CSSSelector::Subselector)
             inRightmost = false;
     }
     return IdMatchingType::None;
@@ -204,7 +204,7 @@ static const CSSSelector* selectorForIdLookup(const ContainerNode& rootNode, con
     for (const CSSSelector* selector = &firstSelector; selector; selector = selector->tagHistory()) {
         if (canBeUsedForIdFastPath(*selector))
             return selector;
-        if (selector->relation() != CSSSelector::SubSelector)
+        if (selector->relation() != CSSSelector::Subselector)
             break;
     }
 
@@ -256,7 +256,7 @@ static ContainerNode& filterRootById(ContainerNode& rootNode, const CSSSelector&
     const CSSSelector* selector = &firstSelector;
     do {
         ASSERT(!canBeUsedForIdFastPath(*selector));
-        if (selector->relation() != CSSSelector::SubSelector)
+        if (selector->relation() != CSSSelector::Subselector)
             break;
         selector = selector->tagHistory();
     } while (selector);
@@ -274,7 +274,7 @@ static ContainerNode& filterRootById(ContainerNode& rootNode, const CSSSelector&
                 }
             }
         }
-        if (selector->relation() == CSSSelector::SubSelector)
+        if (selector->relation() == CSSSelector::Subselector)
             continue;
         if (selector->relation() == CSSSelector::DirectAdjacent || selector->relation() == CSSSelector::IndirectAdjacent)
             inAdjacentChain = true;