Bug 1349553: Account for negations of state-dependent selectors. draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 22 Mar 2017 14:41:22 +0100
changeset 502963 d2b8bfbe1f7d3a20934570a1528d65864cb6b2c8
parent 502962 b086b6db9307ee94c3f376b08fa2b2f4862b2b25
child 503557 00825a6dbfd8bce52cc9a1ec460f2bf9e3c4bd37
push id50427
push userbmo:emilio+bugs@crisal.io
push dateWed, 22 Mar 2017 14:00:40 +0000
bugs1349553
milestone55.0a1
Bug 1349553: Account for negations of state-dependent selectors. MozReview-Commit-ID: VyHuxh9q5N
servo/components/style/gecko/selector_parser.rs
servo/components/style/restyle_hints.rs
--- a/servo/components/style/gecko/selector_parser.rs
+++ b/servo/components/style/gecko/selector_parser.rs
@@ -149,16 +149,19 @@ macro_rules! pseudo_class_name {
                 #[doc = $css]
                 $name,
             )*
             $(
                 #[doc = $s_css]
                 $s_name(Box<[u16]>),
             )*
             /// The non-standard `:-moz-any` pseudo-class.
+            ///
+            /// TODO(emilio): We disallow combinators and pseudos here, so we
+            /// should use SimpleSelector instead
             MozAny(Vec<ComplexSelector<SelectorImpl>>),
         }
     }
 }
 apply_non_ts_list!(pseudo_class_name);
 
 impl ToCss for NonTSPseudoClass {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
--- a/servo/components/style/restyle_hints.rs
+++ b/servo/components/style/restyle_hints.rs
@@ -353,26 +353,32 @@ impl<'a, E> Element for ElementWrapper<'
         match self.snapshot {
             Some(snapshot) if snapshot.has_attrs()
                 => snapshot.each_class(callback),
             _   => self.element.each_class(callback)
         }
     }
 }
 
-/// Returns the union of any `ElementState` flags for components of a `ComplexSelector`
+/// Returns the union of any `ElementState` flags for components of a
+/// `ComplexSelector`.
 pub fn complex_selector_to_state(sel: &ComplexSelector<SelectorImpl>) -> ElementState {
     sel.compound_selector.iter().fold(ElementState::empty(), |state, s| {
         state | selector_to_state(s)
     })
 }
 
 fn selector_to_state(sel: &SimpleSelector<SelectorImpl>) -> ElementState {
     match *sel {
         SimpleSelector::NonTSPseudoClass(ref pc) => SelectorImpl::pseudo_class_state_flag(pc),
+        SimpleSelector::Negation(ref negated) => {
+            negated.iter().fold(ElementState::empty(), |state, s| {
+                state | complex_selector_to_state(s)
+            })
+        }
         _ => ElementState::empty(),
     }
 }
 
 fn is_attr_selector(sel: &SimpleSelector<SelectorImpl>) -> bool {
     match *sel {
         SimpleSelector::ID(_) |
         SimpleSelector::Class(_) |
@@ -497,16 +503,25 @@ impl DependencySet {
         let mut combinator: Option<Combinator> = None;
         loop {
             let mut sensitivities = Sensitivities::new();
             for s in &cur.compound_selector {
                 sensitivities.states.insert(selector_to_state(s));
                 if !sensitivities.attrs {
                     sensitivities.attrs = is_attr_selector(s);
                 }
+
+                // NOTE(emilio): I haven't thought this thoroughly, but we may
+                // not need to do anything for combinators inside negations.
+                //
+                // Or maybe we do, and need to call note_selector recursively
+                // here to account for them correctly, but keep the
+                // sensitivities of the parent?
+                //
+                // In any case, perhaps we should just drop it, see bug 1348802.
             }
             if !sensitivities.is_empty() {
                 self.add_dependency(Dependency {
                     selector: cur.clone(),
                     hint: combinator_to_restyle_hint(combinator),
                     sensitivities: sensitivities,
                 });
             }