Auto merge of #20010 - emilio:pres-hints-cascade, r=bholley

style: Cascade pres hints after normal user rules.

Per https://drafts.csswg.org/css-cascade/#preshint and
https://html.spec.whatwg.org/#presentational-hints.

This was causing failures in the link color reftests with the preferences sheet
as a User sheet.

Bug: 1436782
Reviewed-by: bholley
MozReview-Commit-ID: 9iwEqPBw4CF

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/20010)
<!-- Reviewable:end -->
This commit is contained in:
bors-servo 2018-02-10 07:46:38 -05:00 committed by GitHub
commit 2cc75a783d
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 49 additions and 39 deletions

View file

@ -472,22 +472,28 @@ impl RuleTree {
/// where it likely did not result from a rigorous performance analysis.) /// where it likely did not result from a rigorous performance analysis.)
const RULE_TREE_GC_INTERVAL: usize = 300; const RULE_TREE_GC_INTERVAL: usize = 300;
/// The cascade level these rules are relevant at, as per[1]. /// The cascade level these rules are relevant at, as per[1][2][3].
///
/// Presentational hints for SVG and HTML are in the "author-level
/// zero-specificity" level, that is, right after user rules, and before author
/// rules.
/// ///
/// The order of variants declared here is significant, and must be in /// The order of variants declared here is significant, and must be in
/// _ascending_ order of precedence. /// _ascending_ order of precedence.
/// ///
/// [1]: https://drafts.csswg.org/css-cascade/#cascade-origin /// [1]: https://drafts.csswg.org/css-cascade/#cascade-origin
/// [2]: https://drafts.csswg.org/css-cascade/#preshint
/// [3]: https://html.spec.whatwg.org/multipage/#presentational-hints
#[repr(u8)] #[repr(u8)]
#[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd)] #[derive(Clone, Copy, Debug, Eq, PartialEq, PartialOrd)]
#[cfg_attr(feature = "servo", derive(MallocSizeOf))] #[cfg_attr(feature = "servo", derive(MallocSizeOf))]
pub enum CascadeLevel { pub enum CascadeLevel {
/// Normal User-Agent rules. /// Normal User-Agent rules.
UANormal = 0, UANormal = 0,
/// Presentational hints.
PresHints,
/// User normal rules. /// User normal rules.
UserNormal, UserNormal,
/// Presentational hints.
PresHints,
/// Author normal rules. /// Author normal rules.
AuthorNormal, AuthorNormal,
/// Style attribute normal rules. /// Style attribute normal rules.

View file

@ -1157,7 +1157,10 @@ impl Stylist {
/// Returns the applicable CSS declarations for the given element. /// Returns the applicable CSS declarations for the given element.
/// ///
/// This corresponds to `ElementRuleCollector` in WebKit. /// This corresponds to `ElementRuleCollector` in WebKit, and should push to
/// elements in the list in the order defined by:
///
/// https://drafts.csswg.org/css-cascade/#cascade-origin
pub fn push_applicable_declarations<E, F>( pub fn push_applicable_declarations<E, F>(
&self, &self,
element: E, element: E,
@ -1194,7 +1197,7 @@ impl Stylist {
matches_user_rules && matches_user_rules &&
self.author_styles_enabled == AuthorStylesEnabled::Yes; self.author_styles_enabled == AuthorStylesEnabled::Yes;
// Step 1: Normal user-agent rules. // Normal user-agent rules.
if let Some(map) = self.cascade_data.user_agent.cascade_data.normal_rules(pseudo_element) { if let Some(map) = self.cascade_data.user_agent.cascade_data.normal_rules(pseudo_element) {
map.get_all_matching_rules( map.get_all_matching_rules(
element, element,
@ -1206,8 +1209,33 @@ impl Stylist {
); );
} }
// NB: the following condition, although it may look somewhat
// inaccurate, would be equivalent to something like:
//
// element.matches_user_and_author_rules() ||
// (is_implemented_pseudo &&
// rule_hash_target.matches_user_and_author_rules())
//
// Which may be more what you would probably expect.
if matches_user_rules {
// User normal rules.
if let Some(map) = self.cascade_data.user.normal_rules(pseudo_element) {
map.get_all_matching_rules(
element,
rule_hash_target,
applicable_declarations,
context,
flags_setter,
CascadeLevel::UserNormal,
);
}
}
if pseudo_element.is_none() && !only_default_rules { if pseudo_element.is_none() && !only_default_rules {
// Step 2: Presentational hints. // Presentational hints.
//
// These go before author rules, but after user rules, see:
// https://drafts.csswg.org/css-cascade/#preshint
let length_before_preshints = applicable_declarations.len(); let length_before_preshints = applicable_declarations.len();
element.synthesize_presentational_hints_for_legacy_attributes( element.synthesize_presentational_hints_for_legacy_attributes(
context.visited_handling(), context.visited_handling(),
@ -1222,29 +1250,7 @@ impl Stylist {
} }
} }
// NB: the following condition, although it may look somewhat // XBL / Shadow DOM rules, which are author rules too.
// inaccurate, would be equivalent to something like:
//
// element.matches_user_and_author_rules() ||
// (is_implemented_pseudo &&
// rule_hash_target.matches_user_and_author_rules())
//
// Which may be more what you would probably expect.
if matches_user_rules {
// Step 3a: User normal rules.
if let Some(map) = self.cascade_data.user.normal_rules(pseudo_element) {
map.get_all_matching_rules(
element,
rule_hash_target,
applicable_declarations,
context,
flags_setter,
CascadeLevel::UserNormal,
);
}
}
// Step 3b: XBL / Shadow DOM rules.
// //
// TODO(emilio): Cascade order here is wrong for Shadow DOM. In // TODO(emilio): Cascade order here is wrong for Shadow DOM. In
// particular, normally document rules override ::slotted() rules, but // particular, normally document rules override ::slotted() rules, but
@ -1308,7 +1314,7 @@ impl Stylist {
}); });
if matches_author_rules && !only_default_rules && !cut_off_inheritance { if matches_author_rules && !only_default_rules && !cut_off_inheritance {
// Step 3c: Author normal rules. // Author normal rules.
if let Some(map) = self.cascade_data.author.normal_rules(pseudo_element) { if let Some(map) = self.cascade_data.author.normal_rules(pseudo_element) {
map.get_all_matching_rules( map.get_all_matching_rules(
element, element,
@ -1322,7 +1328,7 @@ impl Stylist {
} }
if !only_default_rules { if !only_default_rules {
// Step 4: Normal style attributes. // Style attribute ("Normal override declarations").
if let Some(sa) = style_attribute { if let Some(sa) = style_attribute {
applicable_declarations.push( applicable_declarations.push(
ApplicableDeclarationBlock::from_declarations( ApplicableDeclarationBlock::from_declarations(
@ -1332,7 +1338,6 @@ impl Stylist {
); );
} }
// Step 5: SMIL override.
// Declarations from SVG SMIL animation elements. // Declarations from SVG SMIL animation elements.
if let Some(so) = smil_override { if let Some(so) = smil_override {
applicable_declarations.push( applicable_declarations.push(
@ -1343,9 +1348,9 @@ impl Stylist {
); );
} }
// Step 6: Animations. // The animations sheet (CSS animations, script-generated
// The animations sheet (CSS animations, script-generated animations, // animations, and CSS transitions that are no longer tied to CSS
// and CSS transitions that are no longer tied to CSS markup) // markup).
if let Some(anim) = animation_rules.0 { if let Some(anim) = animation_rules.0 {
applicable_declarations.push( applicable_declarations.push(
ApplicableDeclarationBlock::from_declarations( ApplicableDeclarationBlock::from_declarations(
@ -1357,13 +1362,12 @@ impl Stylist {
} }
// //
// Steps 7-10 correspond to !important rules, and are handled during // !important rules are handled during rule tree insertion.
// rule tree insertion.
// //
if !only_default_rules { if !only_default_rules {
// Step 11: Transitions. // The transitions sheet (CSS transitions that are tied to CSS
// The transitions sheet (CSS transitions that are tied to CSS markup) // markup).
if let Some(anim) = animation_rules.1 { if let Some(anim) = animation_rules.1 {
applicable_declarations.push( applicable_declarations.push(
ApplicableDeclarationBlock::from_declarations( ApplicableDeclarationBlock::from_declarations(