diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs index 8b95449dd51..0edfbf84451 100644 --- a/components/style/properties/gecko.mako.rs +++ b/components/style/properties/gecko.mako.rs @@ -1924,10 +1924,7 @@ mask-mode mask-repeat mask-clip mask-origin mask-composite mask-position-x mask- self.gecko.mAnimationNameCount = v.len() as u32; for (servo, gecko) in v.zip(self.gecko.mAnimations.iter_mut()) { - let atom = match servo.0 { - None => atom!(""), - Some(ref name) => name.as_atom().clone(), - }; + let atom = servo.0.as_atom().clone(); unsafe { bindings::Gecko_SetAnimationName(gecko, atom.into_addrefed()); } } } @@ -1936,10 +1933,7 @@ mask-mode mask-repeat mask-clip mask-origin mask-composite mask-position-x mask- use crate::properties::longhands::animation_name::single_value::SpecifiedValue as AnimationName; let atom = self.gecko.mAnimations[index].mName.mRawPtr; - if atom == atom!("").as_ptr() { - return AnimationName(None) - } - AnimationName(Some(KeyframesName::from_atom(unsafe { Atom::from_raw(atom) }))) + AnimationName(KeyframesName::from_atom(unsafe { Atom::from_raw(atom) })) } pub fn copy_animation_name_from(&mut self, other: &Self) { self.gecko.mAnimationNameCount = other.gecko.mAnimationNameCount; diff --git a/components/style/properties/properties.mako.rs b/components/style/properties/properties.mako.rs index d0bb6bd1bf6..ff6146ed0d6 100644 --- a/components/style/properties/properties.mako.rs +++ b/components/style/properties/properties.mako.rs @@ -2937,7 +2937,7 @@ pub mod style_structs { /// Returns whether there is any animation specified with /// animation-name other than `none`. pub fn specifies_animations(&self) -> bool { - self.animation_name_iter().any(|name| name.0.is_some()) + self.animation_name_iter().any(|name| !name.is_none()) } /// Returns whether there are any transitions specified. diff --git a/components/style/values/mod.rs b/components/style/values/mod.rs index 9d7b7d74acc..4c36e84d287 100644 --- a/components/style/values/mod.rs +++ b/components/style/values/mod.rs @@ -16,7 +16,6 @@ pub use cssparser::{SourceLocation, Token, RGBA}; use precomputed_hash::PrecomputedHash; use selectors::parser::SelectorParseErrorKind; use std::fmt::{self, Debug, Write}; -use std::hash; use style_traits::{CssWriter, ParseError, StyleParseErrorKind, ToCss}; use to_shmem::impl_trivial_to_shmem; @@ -458,28 +457,33 @@ impl CustomIdent { ident: &CowRcStr<'i>, excluding: &[&str], ) -> Result> { - use crate::properties::CSSWideKeyword; - // https://drafts.csswg.org/css-values-4/#custom-idents: - // - // The CSS-wide keywords are not valid s. The default - // keyword is reserved and is also not a valid . - // - if CSSWideKeyword::from_ident(ident).is_ok() || ident.eq_ignore_ascii_case("default") { + if !Self::is_valid(ident, excluding) { return Err( location.new_custom_error(SelectorParseErrorKind::UnexpectedIdent(ident.clone())) ); } - - // https://drafts.csswg.org/css-values-4/#custom-idents: - // - // Excluded keywords are excluded in all ASCII case permutations. - // if excluding.iter().any(|s| ident.eq_ignore_ascii_case(s)) { Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError)) } else { Ok(CustomIdent(Atom::from(ident.as_ref()))) } } + + fn is_valid(ident: &str, excluding: &[&str]) -> bool { + use crate::properties::CSSWideKeyword; + // https://drafts.csswg.org/css-values-4/#custom-idents: + // + // The CSS-wide keywords are not valid s. The default + // keyword is reserved and is also not a valid . + if CSSWideKeyword::from_ident(ident).is_ok() || ident.eq_ignore_ascii_case("default") { + return false; + } + + // https://drafts.csswg.org/css-values-4/#custom-idents: + // + // Excluded keywords are excluded in all ASCII case permutations. + !excluding.iter().any(|s| ident.eq_ignore_ascii_case(s)) + } } impl ToCss for CustomIdent { @@ -496,45 +500,68 @@ impl ToCss for CustomIdent { /// /// /// +/// +/// We use a single atom for these. Empty atom represents `none` animation. +#[repr(transparent)] #[derive( - Clone, Debug, MallocSizeOf, SpecifiedValueInfo, ToComputedValue, ToResolvedValue, ToShmem, + Clone, Debug, Hash, PartialEq, MallocSizeOf, SpecifiedValueInfo, ToComputedValue, ToResolvedValue, ToShmem, )] -#[repr(C, u8)] -pub enum TimelineOrKeyframesName { - /// - Ident(CustomIdent), - /// - QuotedString(Atom), -} +pub struct TimelineOrKeyframesName(Atom); impl TimelineOrKeyframesName { /// pub fn from_ident(value: &str) -> Self { - let location = SourceLocation { line: 0, column: 0 }; - let custom_ident = CustomIdent::from_ident(location, &value.into(), &["none"]).ok(); - match custom_ident { - Some(ident) => Self::Ident(ident), - None => Self::QuotedString(value.into()), - } + Self(Atom::from(value)) + } + + /// Returns the `none` value. + pub fn none() -> Self { + Self(atom!("")) + } + + /// Returns whether this is the special `none` value. + pub fn is_none(&self) -> bool { + self.0 == atom!("") } /// Create a new TimelineOrKeyframesName from Atom. #[cfg(feature = "gecko")] pub fn from_atom(atom: Atom) -> Self { - debug_assert_ne!(atom, atom!("")); - - // FIXME: We might want to preserve , but currently Gecko - // stores both of and into nsAtom, so - // we can't tell it. - Self::Ident(CustomIdent(atom)) + Self(atom) } /// The name as an Atom pub fn as_atom(&self) -> &Atom { - match *self { - Self::Ident(ref ident) => &ident.0, - Self::QuotedString(ref atom) => atom, + &self.0 + } + + fn parse<'i, 't>(input: &mut Parser<'i, 't>, invalid: &[&str]) -> Result> { + debug_assert!(invalid.contains(&"none")); + let location = input.current_source_location(); + Ok(match *input.next()? { + Token::Ident(ref s) => Self(CustomIdent::from_ident(location, s, invalid)?.0), + Token::QuotedString(ref s) => Self(Atom::from(s.as_ref())), + ref t => return Err(location.new_unexpected_token_error(t.clone())), + }) + } + + fn to_css(&self, dest: &mut CssWriter, invalid: &[&str]) -> fmt::Result + where + W: Write, + { + debug_assert!(invalid.contains(&"none")); + + if self.0 == atom!("") { + return dest.write_str("none") } + + self.0.with_str(|s| { + if CustomIdent::is_valid(s, invalid) { + serialize_identifier(s, dest) + } else { + s.to_css(dest) + } + }) } } @@ -547,53 +574,65 @@ pub trait IsAuto { fn is_auto(&self) -> bool; } -impl PartialEq for TimelineOrKeyframesName { - fn eq(&self, other: &Self) -> bool { - self.as_atom() == other.as_atom() +/// The typedef of . +#[repr(transparent)] +#[derive( + Clone, Debug, Deref, Hash, Eq, PartialEq, MallocSizeOf, SpecifiedValueInfo, ToComputedValue, ToResolvedValue, ToShmem, +)] +pub struct TimelineName(TimelineOrKeyframesName); + +impl TimelineName { + /// Returns the `none` value. + pub fn none() -> Self { + Self(TimelineOrKeyframesName::none()) } } -impl hash::Hash for TimelineOrKeyframesName { - fn hash(&self, state: &mut H) - where - H: hash::Hasher, - { - self.as_atom().hash(state) +impl Parse for TimelineName { + fn parse<'i, 't>(_: &ParserContext, input: &mut Parser<'i, 't>) -> Result> { + Ok(Self(TimelineOrKeyframesName::parse(input, &["none", "auto"])?)) } } -impl Parse for TimelineOrKeyframesName { - fn parse<'i, 't>( - _context: &ParserContext, - input: &mut Parser<'i, 't>, - ) -> Result> { - let location = input.current_source_location(); - match *input.next()? { - Token::Ident(ref s) => Ok(Self::Ident(CustomIdent::from_ident( - location, - s, - &["none"], - )?)), - Token::QuotedString(ref s) => Ok(Self::QuotedString(Atom::from(s.as_ref()))), - ref t => Err(location.new_unexpected_token_error(t.clone())), - } - } -} - -impl ToCss for TimelineOrKeyframesName { +impl ToCss for TimelineName { fn to_css(&self, dest: &mut CssWriter) -> fmt::Result where W: Write, { - match *self { - Self::Ident(ref ident) => ident.to_css(dest), - Self::QuotedString(ref atom) => atom.to_string().to_css(dest), - } + self.0.to_css(dest, &["none", "auto"]) } } -/// The typedef of . -pub type TimelineName = TimelineOrKeyframesName; - /// The typedef of . -pub type KeyframesName = TimelineOrKeyframesName; +#[derive( + Clone, Debug, Deref, Hash, Eq, PartialEq, MallocSizeOf, SpecifiedValueInfo, ToComputedValue, ToResolvedValue, ToShmem, +)] +pub struct KeyframesName(TimelineOrKeyframesName); + +impl KeyframesName { + /// Create a new KeyframesName from Atom. + #[cfg(feature = "gecko")] + pub fn from_atom(atom: Atom) -> Self { + Self(TimelineOrKeyframesName::from_atom(atom)) + } + + /// Returns the `none` value. + pub fn none() -> Self { + Self(TimelineOrKeyframesName::none()) + } +} + +impl Parse for KeyframesName { + fn parse<'i, 't>(_: &ParserContext, input: &mut Parser<'i, 't>) -> Result> { + Ok(Self(TimelineOrKeyframesName::parse(input, &["none"])?)) + } +} + +impl ToCss for KeyframesName { + fn to_css(&self, dest: &mut CssWriter) -> fmt::Result + where + W: Write, + { + self.0.to_css(dest, &["none"]) + } +} diff --git a/components/style/values/specified/box.rs b/components/style/values/specified/box.rs index efdb65ff973..790c6478984 100644 --- a/components/style/values/specified/box.rs +++ b/components/style/values/specified/box.rs @@ -679,33 +679,30 @@ impl AnimationIterationCount { PartialEq, SpecifiedValueInfo, ToComputedValue, + ToCss, ToResolvedValue, ToShmem, )] #[value_info(other_values = "none")] -pub struct AnimationName(pub Option); +pub struct AnimationName(pub KeyframesName); impl AnimationName { /// Get the name of the animation as an `Atom`. pub fn as_atom(&self) -> Option<&Atom> { - self.0.as_ref().map(|n| n.as_atom()) + if self.is_none() { + return None; + } + Some(self.0.as_atom()) } /// Returns the `none` value. pub fn none() -> Self { - AnimationName(None) + AnimationName(KeyframesName::none()) } -} -impl ToCss for AnimationName { - fn to_css(&self, dest: &mut CssWriter) -> fmt::Result - where - W: Write, - { - match self.0 { - Some(ref name) => name.to_css(dest), - None => dest.write_str("none"), - } + /// Returns whether this is the none value. + pub fn is_none(&self) -> bool { + self.0.is_none() } } @@ -715,11 +712,11 @@ impl Parse for AnimationName { input: &mut Parser<'i, 't>, ) -> Result> { if let Ok(name) = input.try_parse(|input| KeyframesName::parse(context, input)) { - return Ok(AnimationName(Some(name))); + return Ok(AnimationName(name)); } input.expect_ident_matching("none")?; - Ok(AnimationName(None)) + Ok(AnimationName(KeyframesName::none())) } } @@ -818,8 +815,6 @@ fn is_default(value: &T) -> bool { pub enum AnimationTimeline { /// Use default timeline. The animation’s timeline is a DocumentTimeline. Auto, - /// The animation is not associated with a timeline. - None, /// The scroll-timeline name Timeline(TimelineName), /// The scroll() notation @@ -849,16 +844,20 @@ impl Parse for AnimationTimeline { input: &mut Parser<'i, 't>, ) -> Result> { // We are using the same parser for TimelineName and KeyframesName, but animation-timeline - // accepts "auto", so need to manually parse this. (We can not derive Parse because - // TimelineName excludes only "none" keyword.) + // accepts "auto", so need to manually parse this. (We can not derive + // Parse because TimelineName excludes only the "none" keyword). + // // FIXME: Bug 1733260: we may drop None based on the spec issue: - // Note: https://github.com/w3c/csswg-drafts/issues/6674. + // https://github.com/w3c/csswg-drafts/issues/6674 + // + // If `none` is removed, then we could potentially shrink this the same + // way we deal with animation-name. if input.try_parse(|i| i.expect_ident_matching("auto")).is_ok() { return Ok(Self::Auto); } if input.try_parse(|i| i.expect_ident_matching("none")).is_ok() { - return Ok(Self::None); + return Ok(AnimationTimeline::Timeline(TimelineName::none())); } // https://drafts.csswg.org/scroll-animations-1/rewrite#scroll-notation