Title: [207536] trunk/Source/WebCore
Revision
207536
Author
hy...@apple.com
Date
2016-10-19 08:07:19 -0700 (Wed, 19 Oct 2016)

Log Message

[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):

Modified Paths

Diff

Modified: trunk/Source/WebCore/ChangeLog (207535 => 207536)


--- trunk/Source/WebCore/ChangeLog	2016-10-19 13:42:43 UTC (rev 207535)
+++ trunk/Source/WebCore/ChangeLog	2016-10-19 15:07:19 UTC (rev 207536)
@@ -1,3 +1,36 @@
+2016-10-18  Dave Hyatt  <hy...@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  <jfernan...@igalia.com>
 
         Web Inspector: Debugger buttons positioned incorrectly, align-content default value is unexpected

Modified: trunk/Source/WebCore/css/CSSSelector.cpp (207535 => 207536)


--- trunk/Source/WebCore/css/CSSSelector.cpp	2016-10-19 13:42:43 UTC (rev 207535)
+++ trunk/Source/WebCore/css/CSSSelector.cpp	2016-10-19 15:07:19 UTC (rev 207536)
@@ -46,6 +46,7 @@
     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 @@
             }
         }
 
-        if (cs->relation() != CSSSelector::SubSelector || !cs->tagHistory())
+        if (cs->relation() != CSSSelector::Subselector || !cs->tagHistory())
             break;
         cs = cs->tagHistory();
     }
@@ -741,7 +742,7 @@
             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;

Modified: trunk/Source/WebCore/css/CSSSelector.h (207535 => 207536)


--- trunk/Source/WebCore/css/CSSSelector.h	2016-10-19 13:42:43 UTC (rev 207535)
+++ trunk/Source/WebCore/css/CSSSelector.h	2016-10-19 15:07:19 UTC (rev 207536)
@@ -81,12 +81,12 @@
             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 @@
         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 @@
         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-*.

Modified: trunk/Source/WebCore/css/CSSSelectorList.cpp (207535 => 207536)


--- trunk/Source/WebCore/css/CSSSelectorList.cpp	2016-10-19 13:42:43 UTC (rev 207535)
+++ trunk/Source/WebCore/css/CSSSelectorList.cpp	2016-10-19 15:07:19 UTC (rev 207536)
@@ -131,9 +131,9 @@
 
 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());
     }

Modified: trunk/Source/WebCore/css/RuleSet.cpp (207535 => 207536)


--- trunk/Source/WebCore/css/RuleSet.cpp	2016-10-19 13:42:43 UTC (rev 207535)
+++ trunk/Source/WebCore/css/RuleSet.cpp	2016-10-19 15:07:19 UTC (rev 207536)
@@ -122,7 +122,7 @@
             }
         }
 
-        if (selector->relation() != CSSSelector::SubSelector)
+        if (selector->relation() != CSSSelector::Subselector)
             matchesRightmostElement = false;
 
         selector = selector->tagHistory();
@@ -268,7 +268,7 @@
             m_slottedPseudoElementRules.append(ruleData);
             return;
         }
-        if (selector->relation() != CSSSelector::SubSelector)
+        if (selector->relation() != CSSSelector::Subselector)
             break;
         selector = selector->tagHistory();
     } while (selector);

Modified: trunk/Source/WebCore/css/SelectorChecker.cpp (207535 => 207536)


--- trunk/Source/WebCore/css/SelectorChecker.cpp	2016-10-19 13:42:43 UTC (rev 207535)
+++ trunk/Source/WebCore/css/SelectorChecker.cpp	2016-10-19 15:07:19 UTC (rev 207536)
@@ -311,7 +311,7 @@
     }
 
     // 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 @@
     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 @@
         };
         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 @@
             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 @@
                 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;

Modified: trunk/Source/WebCore/css/SelectorFilter.cpp (207535 => 207536)


--- trunk/Source/WebCore/css/SelectorFilter.cpp	2016-10-19 13:42:43 UTC (rev 207535)
+++ trunk/Source/WebCore/css/SelectorFilter.cpp	2016-10-19 15:07:19 UTC (rev 207536)
@@ -122,7 +122,7 @@
 {
     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;
@@ -129,7 +129,7 @@
     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;

Modified: trunk/Source/WebCore/css/parser/CSSParser.cpp (207535 => 207536)


--- trunk/Source/WebCore/css/parser/CSSParser.cpp	2016-10-19 13:42:43 UTC (rev 207535)
+++ trunk/Source/WebCore/css/parser/CSSParser.cpp	2016-10-19 15:07:19 UTC (rev 207536)
@@ -12981,10 +12981,10 @@
     }
     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;
 }
 

Modified: trunk/Source/WebCore/css/parser/CSSParserValues.cpp (207535 => 207536)


--- trunk/Source/WebCore/css/parser/CSSParserValues.cpp	2016-10-19 13:42:43 UTC (rev 207535)
+++ trunk/Source/WebCore/css/parser/CSSParserValues.cpp	2016-10-19 15:07:19 UTC (rev 207536)
@@ -447,7 +447,7 @@
     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 @@
     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 @@
     while (end->tagHistory())
         end = end->tagHistory();
 
-    CSSSelector::Relation selectorRelation;
+    CSSSelector::RelationType selectorRelation;
     switch (relation) {
     case CSSParserSelectorCombinator::Child:
         selectorRelation = CSSSelector::Child;
@@ -519,12 +519,12 @@
     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);
 }
 

Modified: trunk/Source/WebCore/css/parser/CSSParserValues.h (207535 => 207536)


--- trunk/Source/WebCore/css/parser/CSSParserValues.h	2016-10-19 13:42:43 UTC (rev 207535)
+++ trunk/Source/WebCore/css/parser/CSSParserValues.h	2016-10-19 15:07:19 UTC (rev 207536)
@@ -226,7 +226,7 @@
     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 @@
     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();

Modified: trunk/Source/WebCore/css/parser/CSSSelectorParser.cpp (207535 => 207536)


--- trunk/Source/WebCore/css/parser/CSSSelectorParser.cpp	2016-10-19 13:42:43 UTC (rev 207535)
+++ trunk/Source/WebCore/css/parser/CSSSelectorParser.cpp	2016-10-19 15:07:19 UTC (rev 207536)
@@ -132,13 +132,12 @@
     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 @@
     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 @@
 
 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 @@
     // 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

Modified: trunk/Source/WebCore/css/parser/CSSSelectorParser.h (207535 => 207536)


--- trunk/Source/WebCore/css/parser/CSSSelectorParser.h	2016-10-19 13:42:43 UTC (rev 207535)
+++ trunk/Source/WebCore/css/parser/CSSSelectorParser.h	2016-10-19 15:07:19 UTC (rev 207536)
@@ -68,7 +68,7 @@
     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&);
 

Modified: trunk/Source/WebCore/cssjit/SelectorCompiler.cpp (207535 => 207536)


--- trunk/Source/WebCore/cssjit/SelectorCompiler.cpp	2016-10-19 13:42:43 UTC (rev 207535)
+++ trunk/Source/WebCore/cssjit/SelectorCompiler.cpp	2016-10-19 15:07:19 UTC (rev 207536)
@@ -398,7 +398,7 @@
     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 @@
         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 @@
             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())

Modified: trunk/Source/WebCore/dom/SelectorQuery.cpp (207535 => 207536)


--- trunk/Source/WebCore/dom/SelectorQuery.cpp	2016-10-19 13:42:43 UTC (rev 207535)
+++ trunk/Source/WebCore/dom/SelectorQuery.cpp	2016-10-19 15:07:19 UTC (rev 207536)
@@ -70,7 +70,7 @@
                 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 @@
     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 @@
     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 @@
                 }
             }
         }
-        if (selector->relation() == CSSSelector::SubSelector)
+        if (selector->relation() == CSSSelector::Subselector)
             continue;
         if (selector->relation() == CSSSelector::DirectAdjacent || selector->relation() == CSSSelector::IndirectAdjacent)
             inAdjacentChain = true;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to