Bug 1406254 - Clear visited rules for text inheritance. r=emilio draft
authorJ. Ryan Stinnett <jryans@gmail.com>
Wed, 11 Oct 2017 14:56:07 -0500
changeset 678825 d3a486d2942436c916f9f712320e1b39cd3c6c68
parent 677000 bbdf62ee058956b87d114a440e37c2f9a31ee7d6
child 678826 5605ac2c31b2d6063d88f27ee0fa1bc861a69a9a
push id84066
push userbmo:jryans@gmail.com
push dateWed, 11 Oct 2017 23:42:38 +0000
reviewersemilio
bugs1406254
milestone58.0a1
Bug 1406254 - Clear visited rules for text inheritance. r=emilio There are two key steps when resolving text styles with `::first-line`: 1. `ResolveStyleForText` computes the initial style of the text via `Servo_ComputedValues_Inherit` 2. `ReparentStyleContext` is called to update style data when the first line of text is moved to be a child of the `::first-line` frame Before this patch, `Servo_ComputedValues_Inherit` would clear out unvisited rules, but visited styles (with rules inside) were cloned as-is, meaning that step 1 might leave the text node with a style that has: * Unvisited rules: None * Visited rules: Some When we later go to step 2 and re-parent onto the `::first-line` styles, we try to cascade with these leftover visited rules. This causes any `::first-line` styles from our parent to be overridden by these rules which are no longer quite right for the new frame tree. In this patch, we resolve this by changing `StyleBuilder::for_inheritance` (which is used by step 1's `Servo_ComputedValues_Inherit`) to also clear out visited rules, so that we use the same logic for both unvisited and visited text styles when reparenting onto the `::first-line` frame. MozReview-Commit-ID: 3sgc4eGHBXs
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/properties.mako.rs
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -246,17 +246,17 @@ impl ops::Deref for ComputedValues {
 impl ops::DerefMut for ComputedValues {
     fn deref_mut(&mut self) -> &mut ComputedValuesInner {
         &mut self.0.mSource
     }
 }
 
 impl ComputedValuesInner {
     /// Clone the visited style.  Used for inheriting parent styles in
-    /// StyleBuilder::for_inheritance.
+    /// StyleBuilder::for_derived_style.
     pub fn clone_visited_style(&self) -> Option<Arc<ComputedValues>> {
         self.visited_style.as_ref().map(|x| x.clone_arc())
     }
 
     #[inline]
     pub fn is_display_contents(&self) -> bool {
         self.get_box().clone_display() == longhands::display::computed_value::T::contents
     }
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -2210,17 +2210,17 @@ impl ComputedValuesInner {
         self.rules.as_ref().unwrap()
     }
 
     /// Whether this style has a -moz-binding value. This is always false for
     /// Servo for obvious reasons.
     pub fn has_moz_binding(&self) -> bool { false }
 
     /// Clone the visited style.  Used for inheriting parent styles in
-    /// StyleBuilder::for_inheritance.
+    /// StyleBuilder::for_derived_style.
     pub fn clone_visited_style(&self) -> Option<Arc<ComputedValues>> {
         self.visited_style.clone()
     }
 
     /// Returns whether this style's display value is equal to contents.
     ///
     /// Since this isn't supported in Servo, this is always false for Servo.
     pub fn is_display_contents(&self) -> bool { false }
@@ -2841,30 +2841,42 @@ impl<'a> StyleBuilder<'a> {
 
     /// Inherits style from the parent element, accounting for the default
     /// computed values that need to be provided as well.
     pub fn for_inheritance(
         device: &'a Device,
         parent: &'a ComputedValues,
         pseudo: Option<<&'a PseudoElement>,
     ) -> Self {
+        // Rebuild the visited style from the parent, ensuring that it will also
+        // not have rules.  This matches the unvisited style that will be
+        // produced by this builder.  This assumes that the caller doesn't need
+        // to adjust or process visited style, so we can just build visited
+        // style here for simplicity.
+        let visited_style = parent.visited_style().map(|style| {
+            Self::for_inheritance(
+                device,
+                style,
+                pseudo,
+            ).build()
+        });
         // FIXME(emilio): This Some(parent) here is inconsistent with what we
         // usually do if `parent` is the default computed values, but that's
         // fine, and we want to eventually get rid of it.
         Self::new(
             device,
             Some(parent),
             Some(parent),
             pseudo,
             CascadeFlags::empty(),
             /* rules = */ None,
             parent.custom_properties().cloned(),
             parent.writing_mode,
             parent.flags,
-            parent.clone_visited_style()
+            visited_style,
         )
     }
 
     /// Returns whether we have a visited style.
     pub fn has_visited_style(&self) -> bool {
         self.visited_style.is_some()
     }