stylo: Fix pseudo element matching for rules in XBL stylesheets (bug 1372876) draft
authorTing-Yu Lin <tlin@mozilla.com>
Thu, 22 Jun 2017 10:57:55 +0800
changeset 598735 128dbf2194ea5314cfb16fc2708537c6a04fa0e6
parent 598703 dc33e00dad90346466fefaa158bc0d79a53668a9
child 634563 102da8bd318aec223c888c8322948b746c3bd351
push id65302
push userbmo:tlin@mozilla.com
push dateThu, 22 Jun 2017 05:24:32 +0000
bugs1372876
milestone56.0a1
stylo: Fix pseudo element matching for rules in XBL stylesheets (bug 1372876) MozReview-Commit-ID: JeliK45G8MT
servo/components/style/dom.rs
servo/components/style/gecko/wrapper.rs
servo/components/style/stylist.rs
--- a/servo/components/style/dom.rs
+++ b/servo/components/style/dom.rs
@@ -590,16 +590,32 @@ pub trait TElement : Eq + PartialEq + De
     /// Returns the anonymous content for the current element's XBL binding,
     /// given if any.
     ///
     /// This is used in Gecko for XBL and shadow DOM.
     fn xbl_binding_anonymous_content(&self) -> Option<Self::ConcreteNode> {
         None
     }
 
+    /// Returns the rule hash target given an element.
+    fn rule_hash_target(&self) -> Self
+    {
+        let is_implemented_pseudo =
+            self.implemented_pseudo_element().is_some();
+
+        // NB: This causes use to rule has pseudo selectors based on the
+        // properties of the originating element (which is fine, given the
+        // find_first_from_right usage).
+        if is_implemented_pseudo {
+            self.closest_non_native_anonymous_ancestor().unwrap()
+        } else {
+            *self
+        }
+    }
+
     /// Gets declarations from XBL bindings from the element. Only gecko element could have this.
     fn get_declarations_from_xbl_bindings<V>(&self,
                                              _pseudo_element: Option<&PseudoElement>,
                                              _applicable_declarations: &mut V)
                                              -> bool
         where V: Push<ApplicableDeclarationBlock> + VecLike<ApplicableDeclarationBlock> {
         false
     }
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -1009,17 +1009,19 @@ impl<'le> TElement for GeckoElement<'le>
     // inheritance.
     fn get_declarations_from_xbl_bindings<V>(&self,
                                              pseudo_element: Option<&PseudoElement>,
                                              applicable_declarations: &mut V)
                                              -> bool
         where V: Push<ApplicableDeclarationBlock> + VecLike<ApplicableDeclarationBlock> {
         // Walk the binding scope chain, starting with the binding attached to our content, up
         // till we run out of scopes or we get cut off.
-        let mut current = Some(*self);
+
+        // If we are NAC, we want to get rules from our rule_hash_target.
+        let mut current = Some(self.rule_hash_target());
 
         while let Some(element) = current {
             if let Some(binding) = element.get_xbl_binding() {
                 binding.get_declarations_for(self,
                                              pseudo_element,
                                              applicable_declarations);
 
                 // If we're not looking at our original element, allow the binding to cut off
--- a/servo/components/style/stylist.rs
+++ b/servo/components/style/stylist.rs
@@ -954,32 +954,16 @@ impl Stylist {
                pseudo_element: Option<&PseudoElement>) -> Option<&PerPseudoElementSelectorMap>
     {
         match pseudo_element {
             Some(pseudo) => self.pseudos_map.get(pseudo),
             None => Some(&self.element_map),
         }
     }
 
-    /// Returns the rule hash target given an element.
-    fn rule_hash_target<E>(&self, element: E) -> E
-        where E: TElement
-    {
-        let is_implemented_pseudo =
-            element.implemented_pseudo_element().is_some();
-
-        // NB: This causes use to rule has pseudo selectors based on the
-        // properties of the originating element (which is fine, given the
-        // find_first_from_right usage).
-        if is_implemented_pseudo {
-            element.closest_non_native_anonymous_ancestor().unwrap()
-        } else {
-            element
-        }
-    }
 
     /// Returns the applicable CSS declarations for the given element by
     /// treating us as an XBL stylesheet-only stylist.
     pub fn push_applicable_declarations_as_xbl_only_stylist<E, V>(&self,
                                                                   element: &E,
                                                                   pseudo_element: Option<&PseudoElement>,
                                                                   applicable_declarations: &mut V)
         where E: TElement,
@@ -988,17 +972,17 @@ impl Stylist {
         let mut matching_context =
             MatchingContext::new(MatchingMode::Normal, None, self.quirks_mode);
         let mut dummy_flag_setter = |_: &E, _: ElementSelectorFlags| {};
 
         let map = match self.get_map(pseudo_element) {
             Some(map) => map,
             None => return,
         };
-        let rule_hash_target = self.rule_hash_target(*element);
+        let rule_hash_target = element.rule_hash_target();
 
         // nsXBLPrototypeResources::ComputeServoStyleSet() added XBL stylesheets under author
         // (doc) level.
         map.author.get_all_matching_rules(element,
                                           &rule_hash_target,
                                           applicable_declarations,
                                           &mut matching_context,
                                           self.quirks_mode,
@@ -1034,17 +1018,17 @@ impl Stylist {
                       style_attribute.is_none() || pseudo_element.is_none(),
                       "Style attributes do not apply to pseudo-elements");
         debug_assert!(pseudo_element.map_or(true, |p| !p.is_precomputed()));
 
         let map = match self.get_map(pseudo_element) {
             Some(map) => map,
             None => return,
         };
-        let rule_hash_target = self.rule_hash_target(*element);
+        let rule_hash_target = element.rule_hash_target();
 
         debug!("Determining if style is shareable: pseudo: {}",
                pseudo_element.is_some());
 
         let only_default_rules = rule_inclusion == RuleInclusion::DefaultOnly;
 
         // Step 1: Normal user-agent rules.
         map.user_agent.get_all_matching_rules(element,
@@ -1096,18 +1080,18 @@ impl Stylist {
                                             CascadeLevel::UserNormal);
             debug!("user normal: {:?}", context.relations);
         } else {
             debug!("skipping user rules");
         }
 
         // Step 3b: XBL rules.
         let cut_off_inheritance =
-            rule_hash_target.get_declarations_from_xbl_bindings(pseudo_element,
-                                                                applicable_declarations);
+            element.get_declarations_from_xbl_bindings(pseudo_element,
+                                                       applicable_declarations);
         debug!("XBL: {:?}", context.relations);
 
         if rule_hash_target.matches_user_and_author_rules() && !only_default_rules {
             // Gecko skips author normal rules if cutting off inheritance.
             // See nsStyleSet::FileRules().
             if !cut_off_inheritance {
                 // Step 3c: Author normal rules.
                 map.author.get_all_matching_rules(element,