From cedc17550cc4a29d4ab8a41dbea4829c196c59b4 Mon Sep 17 00:00:00 2001
From: "J. Ryan Stinnett" <jryans@gmail.com>
Date: Wed, 11 Oct 2017 14:56:07 -0500
Subject: [PATCH] Clear visited rules for text inheritance

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
---
 components/style/properties/gecko.mako.rs      |  2 +-
 components/style/properties/properties.mako.rs | 16 ++++++++++++++--
 2 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs
index e1625ff74910..7831f76aabda 100644
--- a/components/style/properties/gecko.mako.rs
+++ b/components/style/properties/gecko.mako.rs
@@ -251,7 +251,7 @@ impl ops::DerefMut for ComputedValues {
 
 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())
     }
diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs
index de736e7dda5f..328182ef0ae1 100644
--- a/components/style/properties/properties.mako.rs
+++ b/components/style/properties/properties.mako.rs
@@ -2211,7 +2211,7 @@ impl ComputedValuesInner {
     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()
     }
@@ -2842,6 +2842,18 @@ impl<'a> StyleBuilder<'a> {
         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.
@@ -2855,7 +2867,7 @@ impl<'a> StyleBuilder<'a> {
             parent.custom_properties().cloned(),
             parent.writing_mode,
             parent.flags,
-            parent.clone_visited_style()
+            visited_style,
         )
     }