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
This commit is contained in:
J. Ryan Stinnett 2017-10-11 14:56:07 -05:00
parent 882b22b606
commit cedc17550c
2 changed files with 15 additions and 3 deletions

View file

@ -251,7 +251,7 @@ impl ops::DerefMut for ComputedValues {
impl ComputedValuesInner { impl ComputedValuesInner {
/// Clone the visited style. Used for inheriting parent styles in /// 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>> { pub fn clone_visited_style(&self) -> Option<Arc<ComputedValues>> {
self.visited_style.as_ref().map(|x| x.clone_arc()) self.visited_style.as_ref().map(|x| x.clone_arc())
} }

View file

@ -2211,7 +2211,7 @@ impl ComputedValuesInner {
pub fn has_moz_binding(&self) -> bool { false } pub fn has_moz_binding(&self) -> bool { false }
/// Clone the visited style. Used for inheriting parent styles in /// 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>> { pub fn clone_visited_style(&self) -> Option<Arc<ComputedValues>> {
self.visited_style.clone() self.visited_style.clone()
} }
@ -2842,6 +2842,18 @@ impl<'a> StyleBuilder<'a> {
parent: &'a ComputedValues, parent: &'a ComputedValues,
pseudo: Option<<&'a PseudoElement>, pseudo: Option<<&'a PseudoElement>,
) -> Self { ) -> 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 // FIXME(emilio): This Some(parent) here is inconsistent with what we
// usually do if `parent` is the default computed values, but that's // usually do if `parent` is the default computed values, but that's
// fine, and we want to eventually get rid of it. // fine, and we want to eventually get rid of it.
@ -2855,7 +2867,7 @@ impl<'a> StyleBuilder<'a> {
parent.custom_properties().cloned(), parent.custom_properties().cloned(),
parent.writing_mode, parent.writing_mode,
parent.flags, parent.flags,
parent.clone_visited_style() visited_style,
) )
} }