Bug 1379696: Track the selector recursion level, and avoid looking at MatchingMode if the selector isn't the topmost. r?bholley draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 10 Jul 2017 22:29:40 +0200
changeset 606295 7103fa3b6d22d18899800b5ade191df1ad5be6b0
parent 606294 61fd61400f0451b04a7477b229e2c4366167137a
child 606296 c3c1b57f82927732211db4fdf4aa970911f1cd5d
push id67668
push userbmo:emilio+bugs@crisal.io
push dateMon, 10 Jul 2017 20:33:29 +0000
reviewersbholley
bugs1379696
milestone56.0a1
Bug 1379696: Track the selector recursion level, and avoid looking at MatchingMode if the selector isn't the topmost. r?bholley Also, tidy the hover quirk thing to use the same mechanism. MozReview-Commit-ID: KrmNqNyASf6
servo/components/selectors/context.rs
servo/components/selectors/matching.rs
servo/components/style/gecko/wrapper.rs
--- a/servo/components/selectors/context.rs
+++ b/servo/components/selectors/context.rs
@@ -93,16 +93,18 @@ pub struct MatchingContext<'a> {
     /// matching (and also extended after matching).
     pub relations: StyleRelations,
     /// Input with the matching mode we should use when matching selectors.
     pub matching_mode: MatchingMode,
     /// Input with the bloom filter used to fast-reject selectors.
     pub bloom_filter: Option<&'a BloomFilter>,
     /// Input that controls how matching for links is handled.
     pub visited_handling: VisitedHandlingMode,
+    /// The amount of selectors we've been recursing into.
+    pub depth: usize,
     /// Output that records whether we encountered a "relevant link" while
     /// matching _any_ selector for this element. (This differs from
     /// `RelevantLinkStatus` which tracks the status for the _current_ selector
     /// only.)
     pub relevant_link_found: bool,
 
     quirks_mode: QuirksMode,
     classes_and_ids_case_sensitivity: CaseSensitivity,
@@ -116,16 +118,17 @@ impl<'a> MatchingContext<'a> {
                -> Self
     {
         Self {
             relations: StyleRelations::empty(),
             matching_mode: matching_mode,
             bloom_filter: bloom_filter,
             visited_handling: VisitedHandlingMode::AllLinksUnvisited,
             relevant_link_found: false,
+            depth: 0,
             quirks_mode: quirks_mode,
             classes_and_ids_case_sensitivity: quirks_mode.classes_and_ids_case_sensitivity(),
         }
     }
 
     /// Constructs a new `MatchingContext` for use in visited matching.
     pub fn new_for_visited(matching_mode: MatchingMode,
                            bloom_filter: Option<&'a BloomFilter>,
@@ -134,16 +137,17 @@ impl<'a> MatchingContext<'a> {
                            -> Self
     {
         Self {
             relations: StyleRelations::empty(),
             matching_mode: matching_mode,
             bloom_filter: bloom_filter,
             visited_handling: visited_handling,
             relevant_link_found: false,
+            depth: 0,
             quirks_mode: quirks_mode,
             classes_and_ids_case_sensitivity: quirks_mode.classes_and_ids_case_sensitivity(),
         }
     }
 
     /// The quirks mode of the document.
     #[inline]
     pub fn quirks_mode(&self) -> QuirksMode {
--- a/servo/components/selectors/matching.rs
+++ b/servo/components/selectors/matching.rs
@@ -102,16 +102,21 @@ impl<'a, 'b, Impl> LocalMatchingContext<
 
             self.offset = self.selector.len() - selector_iter.selector_length();
         }
     }
 
     /// Returns true if current compound selector matches :active and :hover quirk.
     /// https://quirks.spec.whatwg.org/#the-active-and-hover-quirk
     pub fn active_hover_quirk_matches(&mut self) -> bool {
+        if self.shared.depth != 0 {
+            // Don't allow it in recursive selectors such as :not and :-moz-any.
+            return false;
+        }
+
         if self.shared.quirks_mode() != QuirksMode::Quirks ||
            self.hover_active_quirk_disabled {
             return false;
         }
 
         let mut iter = if self.offset == 0 {
             self.selector.iter()
         } else {
@@ -448,26 +453,28 @@ pub fn matches_complex_selector<E, F>(mu
                                       element: &E,
                                       mut context: &mut LocalMatchingContext<E::Impl>,
                                       flags_setter: &mut F)
                                       -> bool
     where E: Element,
           F: FnMut(&E, ElementSelectorFlags),
 {
     if cfg!(debug_assertions) {
-        if context.shared.matching_mode == MatchingMode::ForStatelessPseudoElement {
+        if context.shared.depth == 0 &&
+            context.shared.matching_mode == MatchingMode::ForStatelessPseudoElement {
             assert!(iter.clone().any(|c| {
                 matches!(*c, Component::PseudoElement(..))
             }));
         }
     }
 
     // If this is the special pseudo-element mode, consume the ::pseudo-element
     // before proceeding, since the caller has already handled that part.
-    if context.shared.matching_mode == MatchingMode::ForStatelessPseudoElement {
+    if context.shared.depth == 0 &&
+        context.shared.matching_mode == MatchingMode::ForStatelessPseudoElement {
         // Consume the pseudo.
         let pseudo = iter.next().unwrap();
         debug_assert!(matches!(*pseudo, Component::PseudoElement(..)),
                       "Used MatchingMode::ForStatelessPseudoElement in a non-pseudo selector");
 
         // The only other parser-allowed Component in this sequence is a state
         // class. We just don't match in that case.
         if let Some(s) = iter.next() {
@@ -728,23 +735,22 @@ fn matches_simple_selector<E, F>(
         Component::LastOfType => {
             matches_generic_nth_child(element, 0, 1, true, true, flags_setter)
         }
         Component::OnlyOfType => {
             matches_generic_nth_child(element, 0, 1, true, false, flags_setter) &&
             matches_generic_nth_child(element, 0, 1, true, true, flags_setter)
         }
         Component::Negation(ref negated) => {
-            let old_value = context.hover_active_quirk_disabled;
-            context.hover_active_quirk_disabled = true;
+            context.shared.depth += 1;
             let result = !negated.iter().all(|ss| {
                 matches_simple_selector(ss, element, context,
                                         relevant_link, flags_setter)
             });
-            context.hover_active_quirk_disabled = old_value;
+            context.shared.depth -= 1;
             result
         }
     }
 }
 
 fn select_name<'a, T>(is_html: bool, local_name: &'a T, local_name_lower: &'a T) -> &'a T {
     if is_html {
         local_name_lower
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -1771,22 +1771,21 @@ impl<'le> ::selectors::Element for Gecko
             NonTSPseudoClass::MozLWThemeDarkText => {
                 self.get_document_theme() == DocumentTheme::Doc_Theme_Dark
             }
             NonTSPseudoClass::MozWindowInactive => {
                 self.document_state().contains(NS_DOCUMENT_STATE_WINDOW_INACTIVE)
             }
             NonTSPseudoClass::MozPlaceholder => false,
             NonTSPseudoClass::MozAny(ref sels) => {
-                let old_value = context.hover_active_quirk_disabled;
-                context.hover_active_quirk_disabled = true;
+                context.shared.depth += 1;
                 let result = sels.iter().any(|s| {
                     matches_complex_selector(s.iter(), self, context, flags_setter)
                 });
-                context.hover_active_quirk_disabled = old_value;
+                context.shared.depth -= 1;
                 result
             }
             NonTSPseudoClass::Lang(ref lang_arg) => {
                 self.match_element_lang(None, lang_arg)
             }
             NonTSPseudoClass::MozSystemMetric(ref s) |
             NonTSPseudoClass::MozLocaleDir(ref s) |
             NonTSPseudoClass::MozEmptyExceptChildrenWithLocalname(ref s) |