Skip to content

Commit

Permalink
Clear visited rules for text inheritance
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jryans committed Oct 11, 2017
1 parent 882b22b commit cedc175
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
2 changes: 1 addition & 1 deletion components/style/properties/gecko.mako.rs
Expand Up @@ -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())
}
Expand Down
16 changes: 14 additions & 2 deletions components/style/properties/properties.mako.rs
Expand Up @@ -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()
}
Expand Down Expand Up @@ -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.
Expand All @@ -2855,7 +2867,7 @@ impl<'a> StyleBuilder<'a> {
parent.custom_properties().cloned(),
parent.writing_mode,
parent.flags,
parent.clone_visited_style()
visited_style,
)
}

Expand Down

0 comments on commit cedc175

Please sign in to comment.