From ce5f1f5c0d98d06270820d14756c1ff1462fa79d Mon Sep 17 00:00:00 2001 From: Brian Marshall Date: Thu, 13 Nov 2014 21:37:42 -0800 Subject: [PATCH] Bug 783213 - Part 2: Don't apply the quirk to selectors that use a pseudo-element or are part of a pseudo-class argument. r=dbaron --- layout/style/nsCSSRuleProcessor.cpp | 98 ++++++++++++++++++++--------- 1 file changed, 70 insertions(+), 28 deletions(-) diff --git a/layout/style/nsCSSRuleProcessor.cpp b/layout/style/nsCSSRuleProcessor.cpp index e5cd7c4fa57d..22a48746a5e4 100644 --- a/layout/style/nsCSSRuleProcessor.cpp +++ b/layout/style/nsCSSRuleProcessor.cpp @@ -53,6 +53,8 @@ #include "mozilla/Preferences.h" #include "mozilla/LookAndFeel.h" #include "mozilla/Likely.h" +#include "mozilla/TypedEnum.h" +#include "mozilla/TypedEnumBits.h" using namespace mozilla; using namespace mozilla::dom; @@ -1429,6 +1431,30 @@ struct NodeMatchContext { } }; +/** + * Additional information about a selector (without combinators) that is + * being matched. + */ +MOZ_BEGIN_ENUM_CLASS(SelectorMatchesFlags, uint8_t) + NONE = 0, + + // The selector's flags are unknown. This happens when you don't know + // if you're starting from the top of a selector. Only used in cases + // where it's acceptable for matching to return a false positive. + // (It's not OK to return a false negative.) + UNKNOWN = 1 << 0, + + // The selector is part of a compound selector which has been split in + // half, where the other half is a pseudo-element. The current + // selector is not a pseudo-element itself. + HAS_PSEUDO_ELEMENT = 1 << 1, + + // The selector is part of an argument to a functional pseudo-class or + // pseudo-element. + IS_PSEUDO_CLASS_ARGUMENT = 1 << 2 +MOZ_END_ENUM_CLASS(SelectorMatchesFlags) +MOZ_MAKE_ENUM_CLASS_BITWISE_OPERATORS(SelectorMatchesFlags) + static bool ValueIncludes(const nsSubstring& aValueList, const nsSubstring& aValue, const nsStringComparator& aComparator) @@ -1460,10 +1486,18 @@ static bool ValueIncludes(const nsSubstring& aValueList, // Return whether the selector matches conditions for the :active and // :hover quirk. -static inline bool ActiveHoverQuirkMatches(nsCSSSelector* aSelector) +static inline bool ActiveHoverQuirkMatches(nsCSSSelector* aSelector, + SelectorMatchesFlags aSelectorFlags) { if (aSelector->HasTagSelector() || aSelector->mAttrList || - aSelector->mIDList || aSelector->mClassList) { + aSelector->mIDList || aSelector->mClassList || + aSelector->IsPseudoElement() || + // Having this quirk means that some selectors will no longer match, + // so it's better to return false when we aren't sure (i.e., the + // flags are unknown). + aSelectorFlags & (SelectorMatchesFlags::UNKNOWN | + SelectorMatchesFlags::HAS_PSEUDO_ELEMENT | + SelectorMatchesFlags::IS_PSEUDO_CLASS_ARGUMENT)) { return false; } @@ -1673,6 +1707,7 @@ StateSelectorMatches(Element* aElement, nsCSSSelector* aSelector, NodeMatchContext& aNodeMatchContext, TreeMatchContext& aTreeMatchContext, + SelectorMatchesFlags aSelectorFlags, bool* const aDependence, EventStates aStatesToCheck) { @@ -1680,18 +1715,11 @@ StateSelectorMatches(Element* aElement, "should only need to call StateSelectorMatches if " "aStatesToCheck is not empty"); - const bool isNegated = aDependence != nullptr; - // Bit-based pseudo-classes if (aStatesToCheck.HasAtLeastOneOfStates(NS_EVENT_STATE_ACTIVE | NS_EVENT_STATE_HOVER) && aTreeMatchContext.mCompatMode == eCompatibility_NavQuirks && - ActiveHoverQuirkMatches(aSelector) && - // This (or the other way around) both make :not() asymmetric - // in quirks mode (and it's hard to work around since we're - // testing the current mNegations, not the first - // (unnegated)). This at least makes it closer to the spec. - !isNegated && + ActiveHoverQuirkMatches(aSelector, aSelectorFlags) && aElement->IsHTML() && !nsCSSRuleProcessor::IsLink(aElement)) { // In quirks mode, only make links sensitive to selectors ":active" // and ":hover". @@ -1727,14 +1755,16 @@ static bool StateSelectorMatches(Element* aElement, nsCSSSelector* aSelector, NodeMatchContext& aNodeMatchContext, - TreeMatchContext& aTreeMatchContext) + TreeMatchContext& aTreeMatchContext, + SelectorMatchesFlags aSelectorFlags) { for (nsPseudoClassList* pseudoClass = aSelector->mPseudoClassList; pseudoClass; pseudoClass = pseudoClass->mNext) { EventStates statesToCheck = sPseudoClassStates[pseudoClass->mType]; if (!statesToCheck.IsEmpty() && !StateSelectorMatches(aElement, aSelector, aNodeMatchContext, - aTreeMatchContext, nullptr, statesToCheck)) { + aTreeMatchContext, aSelectorFlags, nullptr, + statesToCheck)) { return false; } } @@ -1747,11 +1777,11 @@ StateSelectorMatches(Element* aElement, // * what it points to should be set to true whenever a test is skipped // because of aNodeMatchContent.mStateMask static bool SelectorMatches(Element* aElement, - nsCSSSelector* aSelector, - NodeMatchContext& aNodeMatchContext, - TreeMatchContext& aTreeMatchContext, - bool* const aDependence = nullptr) - + nsCSSSelector* aSelector, + NodeMatchContext& aNodeMatchContext, + TreeMatchContext& aTreeMatchContext, + SelectorMatchesFlags aSelectorFlags, + bool* const aDependence = nullptr) { NS_PRECONDITION(!aSelector->IsPseudoElement(), "Pseudo-element snuck into SelectorMatches?"); @@ -1966,8 +1996,9 @@ static bool SelectorMatches(Element* aElement, nsCSSSelector *s = l->mSelectors; NS_ABORT_IF_FALSE(!s->mNext && !s->IsPseudoElement(), "parser failed"); - if (SelectorMatches(aElement, s, aNodeMatchContext, - aTreeMatchContext)) { + if (SelectorMatches( + aElement, s, aNodeMatchContext, aTreeMatchContext, + SelectorMatchesFlags::IS_PSEUDO_CLASS_ARGUMENT)) { break; } } @@ -2229,7 +2260,7 @@ static bool SelectorMatches(Element* aElement, } } else { if (!StateSelectorMatches(aElement, aSelector, aNodeMatchContext, - aTreeMatchContext, aDependence, + aTreeMatchContext, aSelectorFlags, aDependence, statesToCheck)) { return false; } @@ -2319,7 +2350,9 @@ static bool SelectorMatches(Element* aElement, result && negation; negation = negation->mNegations) { bool dependence = false; result = !SelectorMatches(aElement, negation, aNodeMatchContext, - aTreeMatchContext, &dependence); + aTreeMatchContext, + SelectorMatchesFlags::IS_PSEUDO_CLASS_ARGUMENT, + &dependence); // If the selector does match due to the dependence on // aNodeMatchContext.mStateMask, then we want to keep result true // so that the final result of SelectorMatches is true. Doing so @@ -2427,7 +2460,8 @@ static bool SelectorMatchesTree(Element* aPrevElement, aLookForRelevantLink = false; aTreeMatchContext.SetHaveRelevantLink(); } - if (SelectorMatches(element, selector, nodeContext, aTreeMatchContext)) { + if (SelectorMatches(element, selector, nodeContext, aTreeMatchContext, + SelectorMatchesFlags::NONE)) { // to avoid greedy matching, we need to recur if this is a // descendant or general sibling combinator and the next // combinator is different, but we can make an exception for @@ -2509,13 +2543,19 @@ void ContentEnumFunc(const RuleValue& value, nsCSSSelector* aSelector, return; } if (!StateSelectorMatches(pdata->mPseudoElement, aSelector, nodeContext, - data->mTreeMatchContext)) { + data->mTreeMatchContext, + SelectorMatchesFlags::NONE)) { return; } selector = selector->mNext; } + + SelectorMatchesFlags selectorFlags = SelectorMatchesFlags::NONE; + if (aSelector->IsPseudoElement()) { + selectorFlags |= SelectorMatchesFlags::HAS_PSEUDO_ELEMENT; + } if (SelectorMatches(data->mElement, selector, nodeContext, - data->mTreeMatchContext)) { + data->mTreeMatchContext, selectorFlags)) { nsCSSSelector *next = selector->mNext; if (!next || SelectorMatchesTree(data->mElement, next, data->mTreeMatchContext, @@ -2660,6 +2700,7 @@ nsCSSRuleProcessor::HasStateDependentStyle(ElementDependentRuleProcessorData* aD } nsRestyleHint possibleChange = RestyleHintForOp(selector->mOperator); + SelectorMatchesFlags selectorFlags = SelectorMatchesFlags::UNKNOWN; // If hint already includes all the bits of possibleChange, // don't bother calling SelectorMatches, since even if it returns false @@ -2689,9 +2730,9 @@ nsCSSRuleProcessor::HasStateDependentStyle(ElementDependentRuleProcessorData* aD (!isPseudoElement || StateSelectorMatches(aStatefulElement, selectorForPseudo, nodeContext, aData->mTreeMatchContext, - nullptr, aStateMask)) && + selectorFlags, nullptr, aStateMask)) && SelectorMatches(aData->mElement, selector, nodeContext, - aData->mTreeMatchContext) && + aData->mTreeMatchContext, selectorFlags) && SelectorMatchesTree(aData->mElement, selector->mNext, aData->mTreeMatchContext, false)) @@ -2758,7 +2799,7 @@ AttributeEnumFunc(nsCSSSelector* aSelector, AttributeEnumData* aData) NodeMatchContext nodeContext(EventStates(), false); if ((possibleChange & ~(aData->change)) && SelectorMatches(data->mElement, aSelector, nodeContext, - data->mTreeMatchContext) && + data->mTreeMatchContext, SelectorMatchesFlags::UNKNOWN) && SelectorMatchesTree(data->mElement, aSelector->mNext, data->mTreeMatchContext, false)) { aData->change = nsRestyleHint(aData->change | possibleChange); @@ -3575,7 +3616,8 @@ nsCSSRuleProcessor::SelectorListMatches(Element* aElement, NS_ASSERTION(sel, "Should have *some* selectors"); NS_ASSERTION(!sel->IsPseudoElement(), "Shouldn't have been called"); NodeMatchContext nodeContext(EventStates(), false); - if (SelectorMatches(aElement, sel, nodeContext, aTreeMatchContext)) { + if (SelectorMatches(aElement, sel, nodeContext, aTreeMatchContext, + SelectorMatchesFlags::NONE)) { nsCSSSelector* next = sel->mNext; if (!next || SelectorMatchesTree(aElement, next, aTreeMatchContext, false)) {