Bug 1293809 - Don't match native anonymous content to user/author rules. r?emilio draft
authorCameron McCormack <cam@mcc.id.au>
Wed, 23 Nov 2016 15:08:53 +0800
changeset 442795 4fd2e2434e12f20227a35b3a65aece4ca776b22f
parent 442794 55c410abcf38d5e5e392c5cc5d6d3baa45fedd7d
child 537887 6f93d36b63e602483692c04c2c17532af8d87748
push id36812
push userbmo:cam@mcc.id.au
push dateWed, 23 Nov 2016 07:09:52 +0000
reviewersemilio
bugs1293809
milestone53.0a1
Bug 1293809 - Don't match native anonymous content to user/author rules. r?emilio MozReview-Commit-ID: BuAU2TrGt1y
servo/components/style/gecko/wrapper.rs
servo/components/style/matching.rs
servo/components/style/selector_parser.rs
servo/components/style/servo/selector_parser.rs
servo/components/style/stylist.rs
--- a/servo/components/style/gecko/wrapper.rs
+++ b/servo/components/style/gecko/wrapper.rs
@@ -22,16 +22,17 @@ use gecko_bindings::bindings::{Gecko_IsL
 use gecko_bindings::bindings::{Gecko_IsUnvisitedLink, Gecko_IsVisitedLink, Gecko_Namespace};
 use gecko_bindings::bindings::{RawGeckoElement, RawGeckoNode};
 use gecko_bindings::bindings::Gecko_ClassOrClassList;
 use gecko_bindings::bindings::Gecko_GetStyleContext;
 use gecko_bindings::bindings::Gecko_SetNodeFlags;
 use gecko_bindings::bindings::Gecko_StoreStyleDifference;
 use gecko_bindings::structs;
 use gecko_bindings::structs::{NODE_HAS_DIRTY_DESCENDANTS_FOR_SERVO, NODE_IS_DIRTY_FOR_SERVO};
+use gecko_bindings::structs::NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE;
 use gecko_bindings::structs::{nsIAtom, nsIContent, nsStyleContext};
 use parking_lot::RwLock;
 use parser::ParserContextExtraData;
 use properties::{ComputedValues, parse_style_attribute};
 use properties::PropertyDeclarationBlock;
 use selector_parser::ElementExt;
 use selectors::Element;
 use selectors::parser::{AttrSelector, NamespaceConstraint};
@@ -619,9 +620,14 @@ impl<'le> ::selectors::MatchAttr for Gec
     }
 }
 
 impl<'le> ElementExt for GeckoElement<'le> {
     #[inline]
     fn is_link(&self) -> bool {
         self.match_non_ts_pseudo_class(NonTSPseudoClass::AnyLink)
     }
+
+    #[inline]
+    fn matches_user_and_author_rules(&self) -> bool {
+        self.flags() & (NODE_IS_IN_NATIVE_ANONYMOUS_SUBTREE as u32) == 0
+    }
 }
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -101,16 +101,17 @@ pub struct StyleSharingCandidateCache {
 }
 
 #[derive(Clone, Debug)]
 pub enum CacheMiss {
     Parent,
     LocalName,
     Namespace,
     Link,
+    UserAndAuthorRules,
     State,
     IdAttr,
     StyleAttr,
     Class,
     CommonStyleAffectingAttributes,
     PresHints,
     SiblingRules,
     NonCommonAttrRules,
@@ -138,16 +139,20 @@ fn element_matches_candidate<E: TElement
     if *element.get_namespace() != *candidate_element.get_namespace() {
         miss!(Namespace)
     }
 
     if element.is_link() != candidate_element.is_link() {
         miss!(Link)
     }
 
+    if element.matches_user_and_author_rules() != candidate_element.matches_user_and_author_rules() {
+        miss!(UserAndAuthorRules)
+    }
+
     if element.get_state() != candidate_element.get_state() {
         miss!(State)
     }
 
     if element.get_id().is_some() {
         miss!(IdAttr)
     }
 
--- a/servo/components/style/selector_parser.rs
+++ b/servo/components/style/selector_parser.rs
@@ -78,16 +78,18 @@ impl PseudoElementCascadeType {
     #[inline]
     pub fn is_precomputed(&self) -> bool {
         *self == PseudoElementCascadeType::Precomputed
     }
 }
 
 pub trait ElementExt: Element<Impl=TheSelectorImpl> {
     fn is_link(&self) -> bool;
+
+    fn matches_user_and_author_rules(&self) -> bool;
 }
 
 impl TheSelectorImpl {
     #[inline]
     pub fn each_eagerly_cascaded_pseudo_element<F>(mut fun: F)
         where F: FnMut(PseudoElement)
     {
         Self::each_pseudo_element(|pseudo| {
--- a/servo/components/style/servo/selector_parser.rs
+++ b/servo/components/style/servo/selector_parser.rs
@@ -382,9 +382,14 @@ impl MatchAttrGeneric for ServoElementSn
         }.map_or(false, |v| test(v))
     }
 }
 
 impl<E: Element<Impl=TheSelectorImpl>> ElementExt for E {
     fn is_link(&self) -> bool {
         self.match_non_ts_pseudo_class(NonTSPseudoClass::AnyLink)
     }
+
+    #[inline]
+    fn matches_user_and_author_rules(&self) -> bool {
+        true
+    }
 }
--- a/servo/components/style/stylist.rs
+++ b/servo/components/style/stylist.rs
@@ -326,18 +326,19 @@ impl Stylist {
     }
 
     pub fn lazily_compute_pseudo_element_style<E>(&self,
                                                   element: &E,
                                                   pseudo: &PseudoElement,
                                                   parent: &Arc<ComputedValues>)
                                                   -> Option<(Arc<ComputedValues>, StrongRuleNode)>
         where E: Element<Impl=TheSelectorImpl> +
-              fmt::Debug +
-              PresentationalHintsSynthetizer
+                 ElementExt +
+                 fmt::Debug +
+                 PresentationalHintsSynthetizer
     {
         debug_assert!(TheSelectorImpl::pseudo_element_cascade_type(pseudo).is_lazy());
         if self.pseudos_map.get(pseudo).is_none() {
             return None;
         }
 
         let mut declarations = vec![];
 
@@ -414,16 +415,17 @@ impl Stylist {
                                         &self,
                                         element: &E,
                                         parent_bf: Option<&BloomFilter>,
                                         style_attribute: Option<&Arc<RwLock<PropertyDeclarationBlock>>>,
                                         pseudo_element: Option<&PseudoElement>,
                                         applicable_declarations: &mut V,
                                         reason: MatchingReason) -> StyleRelations
         where E: Element<Impl=TheSelectorImpl> +
+                 ElementExt +
                  fmt::Debug +
                  PresentationalHintsSynthetizer,
               V: Push<ApplicableDeclarationBlock> + VecLike<ApplicableDeclarationBlock>
     {
         debug_assert!(!self.is_device_dirty);
         debug_assert!(style_attribute.is_none() || pseudo_element.is_none(),
                       "Style attributes do not apply to pseudo-elements");
         debug_assert!(pseudo_element.is_none() ||
@@ -451,75 +453,79 @@ impl Stylist {
         let length = applicable_declarations.len();
         element.synthesize_presentational_hints_for_legacy_attributes(applicable_declarations);
         if applicable_declarations.len() != length {
             // Never share style for elements with preshints
             relations |= AFFECTED_BY_PRESENTATIONAL_HINTS;
         }
         debug!("preshints: {:?}", relations);
 
-        // Step 3: User and author normal rules.
-        map.user.get_all_matching_rules(element,
-                                        parent_bf,
-                                        applicable_declarations,
-                                        &mut relations,
-                                        reason,
-                                        Importance::Normal);
-        debug!("user normal: {:?}", relations);
-        map.author.get_all_matching_rules(element,
-                                          parent_bf,
-                                          applicable_declarations,
-                                          &mut relations,
-                                          reason,
-                                          Importance::Normal);
-        debug!("author normal: {:?}", relations);
+        if element.matches_user_and_author_rules() {
+            // Step 3: User and author normal rules.
+            map.user.get_all_matching_rules(element,
+                                            parent_bf,
+                                            applicable_declarations,
+                                            &mut relations,
+                                            reason,
+                                            Importance::Normal);
+            debug!("user normal: {:?}", relations);
+            map.author.get_all_matching_rules(element,
+                                              parent_bf,
+                                              applicable_declarations,
+                                              &mut relations,
+                                              reason,
+                                              Importance::Normal);
+            debug!("author normal: {:?}", relations);
 
-        // Step 4: Normal style attributes.
-        if let Some(sa) = style_attribute {
-            if sa.read().any_normal() {
-                relations |= AFFECTED_BY_STYLE_ATTRIBUTE;
-                Push::push(
-                    applicable_declarations,
-                    ApplicableDeclarationBlock::from_declarations(sa.clone(), Importance::Normal));
+            // Step 4: Normal style attributes.
+            if let Some(sa) = style_attribute {
+                if sa.read().any_normal() {
+                    relations |= AFFECTED_BY_STYLE_ATTRIBUTE;
+                    Push::push(
+                        applicable_declarations,
+                        ApplicableDeclarationBlock::from_declarations(sa.clone(), Importance::Normal));
+                }
             }
-        }
 
-        debug!("style attr: {:?}", relations);
+            debug!("style attr: {:?}", relations);
 
-        // Step 5: Author-supplied `!important` rules.
-        map.author.get_all_matching_rules(element,
-                                          parent_bf,
-                                          applicable_declarations,
-                                          &mut relations,
-                                          reason,
-                                          Importance::Important);
+            // Step 5: Author-supplied `!important` rules.
+            map.author.get_all_matching_rules(element,
+                                              parent_bf,
+                                              applicable_declarations,
+                                              &mut relations,
+                                              reason,
+                                              Importance::Important);
 
-        debug!("author important: {:?}", relations);
+            debug!("author important: {:?}", relations);
 
-        // Step 6: `!important` style attributes.
-        if let Some(sa) = style_attribute {
-            if sa.read().any_important() {
-                relations |= AFFECTED_BY_STYLE_ATTRIBUTE;
-                Push::push(
-                    applicable_declarations,
-                    ApplicableDeclarationBlock::from_declarations(sa.clone(), Importance::Important));
+            // Step 6: `!important` style attributes.
+            if let Some(sa) = style_attribute {
+                if sa.read().any_important() {
+                    relations |= AFFECTED_BY_STYLE_ATTRIBUTE;
+                    Push::push(
+                        applicable_declarations,
+                        ApplicableDeclarationBlock::from_declarations(sa.clone(), Importance::Important));
+                }
             }
-        }
+
+            debug!("style attr important: {:?}", relations);
 
-        debug!("style attr important: {:?}", relations);
+            // Step 7: User and UA `!important` rules.
+            map.user.get_all_matching_rules(element,
+                                            parent_bf,
+                                            applicable_declarations,
+                                            &mut relations,
+                                            reason,
+                                            Importance::Important);
 
-        // Step 7: User and UA `!important` rules.
-        map.user.get_all_matching_rules(element,
-                                        parent_bf,
-                                        applicable_declarations,
-                                        &mut relations,
-                                        reason,
-                                        Importance::Important);
-
-        debug!("user important: {:?}", relations);
+            debug!("user important: {:?}", relations);
+        } else {
+            debug!("skipping non-agent rules");
+        }
 
         map.user_agent.get_all_matching_rules(element,
                                               parent_bf,
                                               applicable_declarations,
                                               &mut relations,
                                               reason,
                                               Importance::Important);