selectors: Fix note_next_sequence. draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 25 Jul 2017 23:55:30 +0200
changeset 615396 a65902e659ac0fbf92c038f8c55af41b54f1e2b2
parent 615395 2eadecd3966bf36aa5cfd4835b5f4bed275d809f
child 615397 5120fc8ff24ba7a14d709f52ced52b45f9db4a9b
push id70342
push userbmo:emilio+bugs@crisal.io
push dateTue, 25 Jul 2017 22:17:49 +0000
milestone56.0a1
selectors: Fix note_next_sequence. Selector-matching can backtrack when looking for ancestor combinators, so we can't just arrive there once and forget. Also, there was a further problem before this patch, which was that note_next_sequence was called _before_ checking whether all simple selectors matched, so the sequence you could get there is just wrong. MozReview-Commit-ID: 6g0ibb8EfBU
servo/components/selectors/matching.rs
--- a/servo/components/selectors/matching.rs
+++ b/servo/components/selectors/matching.rs
@@ -88,32 +88,32 @@ impl<'a, 'b, Impl> LocalMatchingContext<
             nesting_level: 0,
             // We flip this off once third sequence is reached.
             hover_active_quirk_disabled: selector.has_pseudo_element(),
         }
     }
 
     /// Updates offset of Selector to show new compound selector.
     /// To be able to correctly re-synthesize main SelectorIter.
-    pub fn note_next_sequence(&mut self, selector_iter: &SelectorIter<Impl>) {
+    fn note_position(&mut self, selector_iter: &SelectorIter<Impl>) {
         if let QuirksMode::Quirks = self.shared.quirks_mode() {
             if self.selector.has_pseudo_element() && self.offset != 0 {
-                // This is the _second_ call to note_next_sequence,
+                // This is the _second_ call to note_position,
                 // which means we've moved past the compound
                 // selector adjacent to the pseudo-element.
                 self.hover_active_quirk_disabled = false;
             }
 
             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 {
+    pub fn active_hover_quirk_matches(&self) -> bool {
         if self.shared.quirks_mode() != QuirksMode::Quirks {
             return false;
         }
 
         // Don't allow it in recursive selectors such as :not and :-moz-any.
         if self.nesting_level != 0 {
             return false;
         }
@@ -489,17 +489,17 @@ pub fn matches_complex_selector<E, F>(mu
                           "Someone messed up pseudo-element parsing");
             return false;
         }
 
         // Advance to the non-pseudo-element part of the selector, and inform the context.
         if iter.next_sequence().is_none() {
             return true;
         }
-        context.note_next_sequence(&mut iter);
+        context.note_position(&iter);
     }
 
     match matches_complex_selector_internal(iter,
                                             element,
                                             context,
                                             &mut RelevantLinkStatus::Looking,
                                             flags_setter) {
         SelectorMatchingResult::Matched => true,
@@ -521,18 +521,16 @@ fn matches_complex_selector_internal<E, 
     let matches_all_simple_selectors = selector_iter.all(|simple| {
         matches_simple_selector(simple, element, context, &relevant_link, flags_setter)
     });
 
     debug!("Matching for {:?}, simple selector {:?}, relevant link {:?}",
            element, selector_iter, relevant_link);
 
     let combinator = selector_iter.next_sequence();
-    // Inform the context that the we've advanced to the next compound selector.
-    context.note_next_sequence(&mut selector_iter);
     let siblings = combinator.map_or(false, |c| c.is_sibling());
     if siblings {
         flags_setter(element, HAS_SLOW_SELECTOR_LATER_SIBLINGS);
     }
 
     if !matches_all_simple_selectors {
         return SelectorMatchingResult::NotMatchedAndRestartFromClosestLaterSibling;
     }
@@ -562,16 +560,19 @@ fn matches_complex_selector_internal<E, 
                 }
             };
 
             loop {
                 let element = match next_element {
                     None => return candidate_not_found,
                     Some(next_element) => next_element,
                 };
+                // Inform the context that the we've advanced to the next
+                // compound selector.
+                context.note_position(&selector_iter);
                 let result = matches_complex_selector_internal(selector_iter.clone(),
                                                                &element,
                                                                context,
                                                                relevant_link,
                                                                flags_setter);
                 match (result, c) {
                     // Return the status immediately.
                     (SelectorMatchingResult::Matched, _) => return result,