style: Don't look at the rule type from value parsing.

This would cause properties to change the value semantics between, e.g.,
@keyframes and non-@keyframes, which would be observable.

It happens not to be observable since the animation-* and transition-*
properties are not allowed in @keyframes, nor have bits in `contain`, and none
of the two properties are allowed in @page. But I think it's the right thing to
do.

This still causes a quirk like a property value in chrome / user origins being
potentially different if the value is specified via CSS var functions. But I
think that is fine.

Bug: 1466136
Reviewed-by: hiro
MozReview-Commit-ID: GhoPt0I34oO
This commit is contained in:
Emilio Cobos Álvarez 2018-06-01 17:11:00 +02:00
parent 710184b670
commit f829300751
No known key found for this signature in database
GPG key ID: 056B727BB9C1027C
2 changed files with 43 additions and 4 deletions

View file

@ -499,6 +499,11 @@ impl NonCustomPropertyId {
_ => {} _ => {}
} }
self.allowed_in_ignoring_rule_type(context)
}
fn allowed_in_ignoring_rule_type(self, context: &ParserContext) -> bool {
// The semantics of these are kinda hard to reason about, what follows // The semantics of these are kinda hard to reason about, what follows
// is a description of the different combinations that can happen with // is a description of the different combinations that can happen with
// these three sets. // these three sets.
@ -1423,11 +1428,17 @@ impl UnparsedValue {
// As of this writing, only the base URL is used for property // As of this writing, only the base URL is used for property
// values. // values.
// //
// FIXME(emilio): These bits are slightly fishy. // NOTE(emilio): we intentionally pase `None` as the rule type here.
// If something starts depending on it, it's probably a bug, since
// it'd change how values are parsed depending on whether we're in a
// @keyframes rule or not, for example... So think twice about
// whether you want to do this!
//
// FIXME(emilio): ParsingMode is slightly fishy...
let context = ParserContext::new( let context = ParserContext::new(
Origin::Author, Origin::Author,
&self.url_data, &self.url_data,
Some(CssRuleType::Style), None,
ParsingMode::DEFAULT, ParsingMode::DEFAULT,
quirks_mode, quirks_mode,
); );
@ -1674,6 +1685,24 @@ impl PropertyId {
Ok(id) Ok(id)
} }
/// Parses a property name, and returns an error if it's unknown or isn't
/// allowed in this context, ignoring the rule_type checks.
///
/// This is useful for parsing stuff from CSS values, for example.
#[inline]
pub fn parse_ignoring_rule_type(
name: &str,
context: &ParserContext,
) -> Result<Self, ()> {
let id = Self::parse_unchecked(name)?;
if !id.allowed_in_ignoring_rule_type(context) {
return Err(());
}
Ok(id)
}
/// Returns a property id from Gecko's nsCSSPropertyID. /// Returns a property id from Gecko's nsCSSPropertyID.
#[cfg(feature = "gecko")] #[cfg(feature = "gecko")]
#[allow(non_upper_case_globals)] #[allow(non_upper_case_globals)]
@ -1787,6 +1816,16 @@ impl PropertyId {
id.allowed_in(context) id.allowed_in(context)
} }
#[inline]
fn allowed_in_ignoring_rule_type(&self, context: &ParserContext) -> bool {
let id = match self.non_custom_id() {
// Custom properties are allowed everywhere
None => return true,
Some(id) => id,
};
id.allowed_in_ignoring_rule_type(context)
}
/// Whether the property supports the given CSS type. /// Whether the property supports the given CSS type.
/// `ty` should a bitflags of constants in style_traits::CssType. /// `ty` should a bitflags of constants in style_traits::CssType.
pub fn supports_type(&self, ty: u8) -> bool { pub fn supports_type(&self, ty: u8) -> bool {

View file

@ -465,7 +465,7 @@ fn change_bits_for_longhand(longhand: LonghandId) -> WillChangeBits {
} }
fn change_bits_for_maybe_property(ident: &str, context: &ParserContext) -> WillChangeBits { fn change_bits_for_maybe_property(ident: &str, context: &ParserContext) -> WillChangeBits {
let id = match PropertyId::parse(ident, context) { let id = match PropertyId::parse_ignoring_rule_type(ident, context) {
Ok(id) => id, Ok(id) => id,
Err(..) => return WillChangeBits::empty(), Err(..) => return WillChangeBits::empty(),
}; };
@ -777,7 +777,7 @@ impl Parse for TransitionProperty {
let location = input.current_source_location(); let location = input.current_source_location();
let ident = input.expect_ident()?; let ident = input.expect_ident()?;
let id = match PropertyId::parse(&ident, context) { let id = match PropertyId::parse_ignoring_rule_type(&ident, context) {
Ok(id) => id, Ok(id) => id,
Err(..) => return Ok(TransitionProperty::Unsupported( Err(..) => return Ok(TransitionProperty::Unsupported(
CustomIdent::from_ident(location, ident, &["none"])?, CustomIdent::from_ident(location, ident, &["none"])?,