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
--- 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()
}