servo: Merge #17867 - style: Don't skip computation of pseudo-elements of display: none elements (from emilio:pseudo-display-none); r=heycam
authorEmilio Cobos Álvarez <emilio@crisal.io>
Wed, 26 Jul 2017 02:15:30 -0700
changeset 419781 70fa8180a556ae6f5aad0031770b7a78ce24012f
parent 419780 ceac50856fee7fc34e6788b967eb2363bbd12aa8
child 419782 53c50961ee5cafc5ed5d84b0f3ec6377b4c0a938
push id7566
push usermtabara@mozilla.com
push dateWed, 02 Aug 2017 08:25:16 +0000
treeherdermozilla-beta@86913f512c3c [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewersheycam
milestone56.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
servo: Merge #17867 - style: Don't skip computation of pseudo-elements of display: none elements (from emilio:pseudo-display-none); r=heycam We have optimizations to avoid doing selector-matching when the style attribute changes, so, given you can toggle the display property and the pseudo-elements will suddenly become effective, we can't really skip them. Furthermore, we assume that if an element has an ElementStyles, they're up-to-date and we can use them for getComputedStyle, so it's pretty easy to prove that we do the wrong thing when calling getComputedStyle with a pseudo-element on a display: none root. Bug: 1384065 Reviewed-by: heycam MozReview-Commit-ID: BIOqevGZyrm Source-Repo: https://github.com/servo/servo Source-Revision: f61528d2977b11673af30e075b4556a2a30f5aaf
servo/components/style/style_resolver.rs
--- a/servo/components/style/style_resolver.rs
+++ b/servo/components/style/style_resolver.rs
@@ -135,29 +135,20 @@ where
 
 
     /// Resolve the style of a given element, and all its eager pseudo-elements.
     pub fn resolve_style(
         &mut self,
         parent_style: Option<&ComputedValues>,
         layout_parent_style: Option<&ComputedValues>,
     ) -> ElementStyles {
-        use properties::longhands::display::computed_value::T as display;
-
         let primary_style =
             self.resolve_primary_style(parent_style, layout_parent_style);
 
         let mut pseudo_styles = EagerPseudoStyles::default();
-        if primary_style.style.get_box().clone_display() == display::none {
-            return ElementStyles {
-                // FIXME(emilio): Remove the Option<>.
-                primary: Some(primary_style.style),
-                pseudos: pseudo_styles,
-            }
-        }
 
         if self.element.implemented_pseudo_element().is_none() {
             let layout_parent_style_for_pseudo =
                 if primary_style.style.is_display_contents() {
                     layout_parent_style
                 } else {
                     Some(&*primary_style.style)
                 };
@@ -232,46 +223,36 @@ where
         )
     }
 
     /// Cascade the element and pseudo-element styles with the default parents.
     pub fn cascade_styles_with_default_parents(
         &mut self,
         inputs: ElementCascadeInputs,
     ) -> ElementStyles {
-        use properties::longhands::display::computed_value::T as display;
         with_default_parent_styles(self.element, move |parent_style, layout_parent_style| {
             let primary_style = PrimaryStyle {
                 style: self.cascade_style_and_visited(
                    inputs.primary,
                    parent_style,
                    layout_parent_style,
                    /* pseudo = */ None,
                 ),
             };
 
             let mut pseudo_styles = EagerPseudoStyles::default();
-            let pseudo_array = inputs.pseudos.into_array();
-            if pseudo_array.is_none() ||
-                primary_style.style.get_box().clone_display() == display::none {
-                return ElementStyles {
-                    primary: Some(primary_style.style),
-                    pseudos: pseudo_styles,
-                }
-            }
-
-            {
+            if let Some(mut pseudo_array) = inputs.pseudos.into_array() {
                 let layout_parent_style_for_pseudo =
                     if primary_style.style.is_display_contents() {
                         layout_parent_style
                     } else {
                         Some(&*primary_style.style)
                     };
 
-                for (i, mut inputs) in pseudo_array.unwrap().iter_mut().enumerate() {
+                for (i, mut inputs) in pseudo_array.iter_mut().enumerate() {
                     if let Some(inputs) = inputs.take() {
                         let pseudo = PseudoElement::from_eager_index(i);
                         pseudo_styles.set(
                             &pseudo,
                             self.cascade_style_and_visited(
                                 inputs,
                                 Some(&*primary_style.style),
                                 layout_parent_style_for_pseudo,