servo: Merge #16093 - Bug 1349553: Account for negations of state-dependent selectors (from emilio:state); r=bholley,heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Thu, 23 Mar 2017 01:52:43 -0700
changeset 504049 e0546fc74c52c8b708e9e5b1a238bba0a1b241e6
parent 504048 7e711ee19284d9343b5de46b6252943702ac42d7
child 504050 9b6a70c439371685d1ac83cf7b6f7ac7a419804a
push id50739
push userbmo:emilio+bugs@crisal.io
push dateThu, 23 Mar 2017 23:47:46 +0000
reviewersbholley, heycam
bugs1349553
milestone55.0a1
servo: Merge #16093 - Bug 1349553: Account for negations of state-dependent selectors (from emilio:state); r=bholley,heycam Source-Repo: https://github.com/servo/servo Source-Revision: 301ba366b88821bd85a2a5b700d4671ab3958aca
servo/components/style/gecko/selector_parser.rs
servo/components/style/restyle_hints.rs
servo/components/style/traversal.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,
                 });
             }
--- a/servo/components/style/traversal.rs
+++ b/servo/components/style/traversal.rs
@@ -562,16 +562,17 @@ fn compute_style<E, D>(_traversal: &D,
 fn preprocess_children<E, D>(traversal: &D,
                              element: E,
                              mut propagated_hint: StoredRestyleHint,
                              damage_handled: RestyleDamage,
                              parent_inherited_style_changed: bool)
     where E: TElement,
           D: DomTraversal<E>
 {
+    trace!("preprocess_children: {:?}", element);
     // Loop over all the children.
     for child in element.as_node().children() {
         // FIXME(bholley): Add TElement::element_children instead of this.
         let child = match child.as_element() {
             Some(el) => el,
             None => continue,
         };
 
@@ -594,16 +595,18 @@ fn preprocess_children<E, D>(traversal: 
         // Propagate the parent and sibling restyle hint.
         if !propagated_hint.is_empty() {
             restyle_data.hint.insert(&propagated_hint);
         }
 
         // Handle element snapshots and sibling restyle hints.
         let stylist = &traversal.shared_context().stylist;
         let later_siblings = restyle_data.compute_final_hint(child, stylist);
+        trace!(" > {:?} -> {:?}, later_siblings: {:?}",
+               child, restyle_data.hint, later_siblings);
         if later_siblings {
             propagated_hint.insert(&(RESTYLE_SELF | RESTYLE_DESCENDANTS).into());
         }
 
         // Store the damage already handled by ancestors.
         restyle_data.set_damage_handled(damage_handled);
 
         // If properties that we inherited from the parent changed, we need to recascade.