Bug 1376352: properly handle ::before/::after rules applying to replaced elements. r?heycam draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Mon, 03 Jul 2017 09:39:46 +0200
changeset 603251 0d2ee6465107e4507029ad909804c56049680c26
parent 603248 afa14374f151d38134f7223984d9f1427e0ab2fb
child 603252 44aad1eebc8dbf263fc967ab4e5cf7b74215b386
child 603279 a057b5dbe0f79532666df2b6d645acff6a5bfff0
push id66705
push userbmo:emilio+bugs@crisal.io
push dateMon, 03 Jul 2017 09:28:23 +0000
reviewersheycam
bugs1376352
milestone56.0a1
Bug 1376352: properly handle ::before/::after rules applying to replaced elements. r?heycam MozReview-Commit-ID: FO0TyWsPPG7
servo/components/style/matching.rs
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -1571,33 +1571,31 @@ pub trait MatchMethods : TElement {
         // This happens with display:none elements, and not-yet-existing
         // pseudo-elements.
         if new_style_is_display_none && old_style_is_display_none {
             // The style remains display:none. No need for damage.
             return StyleDifference::new(RestyleDamage::empty(), StyleChange::Unchanged)
         }
 
         if pseudo.map_or(false, |p| p.is_before_or_after()) {
-            if (old_style_is_display_none ||
-                old_values.ineffective_content_property()) &&
-               (new_style_is_display_none ||
-                new_values.ineffective_content_property()) {
-                // The pseudo-element will remain undisplayed, so just avoid
-                // triggering any change.
-                return StyleDifference::new(RestyleDamage::empty(), StyleChange::Unchanged)
+            let old_style_generates_no_pseudo =
+                old_style_is_display_none ||
+                old_values.ineffective_content_property();
+
+            let new_style_generates_no_pseudo =
+                new_style_is_display_none ||
+                new_values.ineffective_content_property();
+
+            if old_style_generates_no_pseudo != new_style_generates_no_pseudo {
+                return StyleDifference::new(RestyleDamage::reconstruct(), StyleChange::Changed)
             }
-            // FIXME(bz): This will keep reframing replaced elements.  Do we
-            // need this at all?  Seems like if we add/remove ::before or
-            // ::after styles we would get reframed over in match_pseudos and if
-            // that part didn't change and we had no frame for the
-            // ::before/::after then we don't care.  Need to double-check that
-            // we handle the "content" and "display" properties changing
-            // correctly, though.
-            // https://bugzilla.mozilla.org/show_bug.cgi?id=1376352
-            return StyleDifference::new(RestyleDamage::reconstruct(), StyleChange::Changed)
+
+            // The pseudo-element will remain undisplayed, so just avoid
+            // triggering any change.
+            return StyleDifference::new(RestyleDamage::empty(), StyleChange::Unchanged)
         }
 
         if pseudo.map_or(false, |p| p.is_first_letter()) {
             // No one cares about this pseudo, and we've checked above that
             // we're not switching from a "cares" to a "doesn't care" state
             // or vice versa.
             return StyleDifference::new(RestyleDamage::empty(),
                                         StyleChange::Unchanged)