Mirrors/servo
1
0
Fork 0
mirror of https://github.com/servo/servo.git synced 2025-08-16 02:45:36 +01:00
Code Issues Projects Releases Packages Wiki Activity

style: Simplify the implementation of HasAuthorSpecifiedRules.

Browse source 
This patch computes the author-specified properties during the CSS cascade, and
removes the complex rule-tree-based implementation that tries to do the cascade
again.

This changes behavior in two ways, one of them which is not observable to
content, I believe:

 * revert now re-enables the native styling. This was brought up in
   https://github.com/w3c/csswg-drafts/issues/4777 and I think it is a bug-fix.

   This is observable to content, and I'm adding a test for it.

 * We don't look at inherited styles from our ancestors when `inherit` is
   specified in a non-author stylesheet. This was introduced for bug 452969 but
   we don't seem to inherit background anymore for file controls or such. It
   seems back then file controls used to have a text-field.

   I audited forms.css and ua.css and we don't explicitly inherit
   padding / border / background-color into any nested form control.

We keep the distinction between border/background and padding, because the later
has some callers. I think we should try to align with Chromium in the long run
and remove the padding bit.

We need to give an appearance to the range-thumb and such so that we can assert
that we don't call HasAuthorSpecifiedRules on non-themed stuff. I used a new
internal value for that.

Differential Revision: https://phabricator.services.mozilla.com/D67722
This commit is contained in:
Emilio Cobos Álvarez 2020-03-26 16:48:01 +00:00
parent 7d438cd816
commit 414edb5a4a
5 changed files with 122 additions and 243 deletions
Download patch file Download diff file Expand all files Collapse all files

33
components/style/properties/cascade.rs
View file

@ -13,7 +13,7 @@ use crate::media_queries::Device;
use crate::properties::{ComputedValues, StyleBuilder};
use crate::properties::{LonghandId, LonghandIdSet, CSSWideKeyword};
use crate::properties::{PropertyDeclaration, PropertyDeclarationId, DeclarationImportanceIterator};
use crate::properties::CASCADE_PROPERTY;
use crate::properties::{CASCADE_PROPERTY, ComputedValueFlags};
use crate::rule_cache::{RuleCache, RuleCacheConditions};
use crate::rule_tree::StrongRuleNode;
use crate::selector_parser::PseudoElement;
@ -411,6 +411,7 @@ struct Cascade<'a, 'b: 'a> {
context: &'a mut computed::Context<'b>,
cascade_mode: CascadeMode<'a>,
seen: LonghandIdSet,
author_specified: LonghandIdSet,
reverted: PerOrigin<LonghandIdSet>,
}
@ -420,6 +421,7 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
context,
cascade_mode,
seen: LonghandIdSet::default(),
author_specified: LonghandIdSet::default(),
reverted: Default::default(),
}
}
@ -557,6 +559,9 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
}
self.seen.insert(physical_longhand_id);
if origin == Origin::Author {
self.author_specified.insert(physical_longhand_id);
}
let unset = css_wide_keyword.map_or(false, |css_wide_keyword| {
match css_wide_keyword {
@ -679,6 +684,14 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
if let Some(svg) = builder.get_svg_if_mutated() {
svg.fill_arrays();
}
}
if self.author_specified.contains_any(LonghandIdSet::border_background_properties()) {
builder.add_flags(ComputedValueFlags::HAS_AUTHOR_SPECIFIED_BORDER_BACKGROUND);
}
if self.author_specified.contains_any(LonghandIdSet::padding_properties()) {
builder.add_flags(ComputedValueFlags::HAS_AUTHOR_SPECIFIED_PADDING);
}
#[cfg(feature = "servo")]
@ -699,12 +712,26 @@ impl<'a, 'b: 'a> Cascade<'a, 'b> {
None => return false,
};
let cached_style = match cache.find(guards, &self.context.builder) {
let builder = &mut self.context.builder;
let cached_style = match cache.find(guards, &builder) {
Some(style) => style,
None => return false,
};
self.context.builder.copy_reset_from(cached_style);
builder.copy_reset_from(cached_style);
// We're using the same reset style as another element, and we'll skip
// applying the relevant properties. So we need to do the relevant
// bookkeeping here to keep these two bits correct.
//
// Note that all the properties involved are non-inherited, so we don't
// need to do anything else other than just copying the bits over.
let reset_props_bits =
ComputedValueFlags::HAS_AUTHOR_SPECIFIED_BORDER_BACKGROUND |
ComputedValueFlags::HAS_AUTHOR_SPECIFIED_PADDING;
builder.add_flags(cached_style.flags & reset_props_bits);
true
}

14
components/style/properties/computed_value_flags.rs
View file

@ -70,6 +70,20 @@ bitflags! {
/// Whether this element is inside an `opacity: 0` subtree.
const IS_IN_OPACITY_ZERO_SUBTREE = 1 << 12;
/// Whether there are author-specified rules for border-* properties
/// (except border-image-*), background-color, or background-image.
///
/// TODO(emilio): Maybe do include border-image, see:
///
/// https://github.com/w3c/csswg-drafts/issues/4777#issuecomment-604424845
const HAS_AUTHOR_SPECIFIED_BORDER_BACKGROUND = 1 << 13;
/// Whether there are author-specified rules for padding-* properties.
///
/// FIXME(emilio): Try to merge this with BORDER_BACKGROUND, see
/// https://github.com/w3c/csswg-drafts/issues/4777
const HAS_AUTHOR_SPECIFIED_PADDING = 1 << 14;
}
}

114
components/style/properties/properties.mako.rs
View file

@ -750,6 +750,52 @@ static ${name}: LonghandIdSet = LonghandIdSet {
};
</%def>
<%
logical_groups = defaultdict(list)
for prop in data.longhands:
if prop.logical_group:
logical_groups[prop.logical_group].append(prop)
for group, props in logical_groups.iteritems():
logical_count = sum(1 for p in props if p.logical)
if logical_count * 2 != len(props):
raise RuntimeError("Logical group {} has ".format(group) +
"unbalanced logical / physical properties")
FIRST_LINE_RESTRICTIONS = PropertyRestrictions.first_line(data)
FIRST_LETTER_RESTRICTIONS = PropertyRestrictions.first_letter(data)
MARKER_RESTRICTIONS = PropertyRestrictions.marker(data)
PLACEHOLDER_RESTRICTIONS = PropertyRestrictions.placeholder(data)
CUE_RESTRICTIONS = PropertyRestrictions.cue(data)
def restriction_flags(property):
name = property.name
flags = []
if name in FIRST_LINE_RESTRICTIONS:
flags.append("APPLIES_TO_FIRST_LINE")
if name in FIRST_LETTER_RESTRICTIONS:
flags.append("APPLIES_TO_FIRST_LETTER")
if name in PLACEHOLDER_RESTRICTIONS:
flags.append("APPLIES_TO_PLACEHOLDER")
if name in MARKER_RESTRICTIONS:
flags.append("APPLIES_TO_MARKER")
if name in CUE_RESTRICTIONS:
flags.append("APPLIES_TO_CUE")
return flags
%>
/// A group for properties which may override each other
/// via logical resolution.
#[derive(Clone, Copy, Eq, Hash, PartialEq)]
pub enum LogicalGroup {
% for group in logical_groups.iterkeys():
/// ${group}
${to_camel_case(group)},
% endfor
}
/// A set of longhand properties
#[derive(Clone, Copy, Debug, Default, MallocSizeOf, PartialEq)]
pub struct LonghandIdSet {
@ -837,6 +883,29 @@ impl LonghandIdSet {
&HAS_NO_EFFECT_ON_SCROLLBARS
}
/// Returns the set of padding properties for the purpose of disabling
/// native appearance.
#[inline]
pub fn padding_properties() -> &'static Self {
<% assert "padding" in logical_groups %>
${static_longhand_id_set(
"PADDING_PROPERTIES",
lambda p: p.logical_group == "padding"
)}
&PADDING_PROPERTIES
}
/// Returns the set of border properties for the purpose of disabling native
/// appearance.
#[inline]
pub fn border_background_properties() -> &'static Self {
${static_longhand_id_set(
"BORDER_BACKGROUND_PROPERTIES",
lambda p: (p.logical_group and p.logical_group.startswith("border")) or p.name in ["background-color", "background-image"]
)}
&BORDER_BACKGROUND_PROPERTIES
}
/// Iterate over the current longhand id set.
pub fn iter(&self) -> LonghandIdSetIterator {
LonghandIdSetIterator { longhands: self, cur: 0, }
@ -998,51 +1067,6 @@ bitflags! {
}
}
<%
logical_groups = defaultdict(list)
for prop in data.longhands:
if prop.logical_group:
logical_groups[prop.logical_group].append(prop)
for group, props in logical_groups.iteritems():
logical_count = sum(1 for p in props if p.logical)
if logical_count * 2 != len(props):
raise RuntimeError("Logical group {} has ".format(group) +
"unbalanced logical / physical properties")
FIRST_LINE_RESTRICTIONS = PropertyRestrictions.first_line(data)
FIRST_LETTER_RESTRICTIONS = PropertyRestrictions.first_letter(data)
MARKER_RESTRICTIONS = PropertyRestrictions.marker(data)
PLACEHOLDER_RESTRICTIONS = PropertyRestrictions.placeholder(data)
CUE_RESTRICTIONS = PropertyRestrictions.cue(data)
def restriction_flags(property):
name = property.name
flags = []
if name in FIRST_LINE_RESTRICTIONS:
flags.append("APPLIES_TO_FIRST_LINE")
if name in FIRST_LETTER_RESTRICTIONS:
flags.append("APPLIES_TO_FIRST_LETTER")
if name in PLACEHOLDER_RESTRICTIONS:
flags.append("APPLIES_TO_PLACEHOLDER")
if name in MARKER_RESTRICTIONS:
flags.append("APPLIES_TO_MARKER")
if name in CUE_RESTRICTIONS:
flags.append("APPLIES_TO_CUE")
return flags
%>
/// A group for properties which may override each other
/// via logical resolution.
#[derive(Clone, Copy, Eq, Hash, PartialEq)]
pub enum LogicalGroup {
% for group in logical_groups.iterkeys():
/// ${group}
${to_camel_case(group)},
% endfor
}
/// An identifier for a given longhand property.
#[derive(Clone, Copy, Eq, Hash, MallocSizeOf, PartialEq, ToComputedValue, ToResolvedValue, ToShmem)]
#[repr(u16)]