style: cleanup animation-name

Make the representation the same between Gecko and Servo code. This will
enable further clean-ups in the future.

Make serialization be correct, serializing as identifier unless it's an
invalid one (in which case we serialize as a string).

This changes our stringification behavior in the specified style, but
now it will match the computed style and be more correct over-all.

Differential Revision: https://phabricator.services.mozilla.com/D144473
This commit is contained in:
Emilio Cobos Álvarez 2023-08-11 02:30:40 +02:00 committed by Martin Robinson
parent ca25d17626
commit 679a69defb
4 changed files with 134 additions and 102 deletions

View file

@ -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;

View file

@ -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.

View file

@ -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<Self, ParseError<'i>> {
use crate::properties::CSSWideKeyword;
// https://drafts.csswg.org/css-values-4/#custom-idents:
//
// The CSS-wide keywords are not valid <custom-ident>s. The default
// keyword is reserved and is also not a valid <custom-ident>.
//
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 <custom-ident>s. The default
// keyword is reserved and is also not a valid <custom-ident>.
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 {
///
/// <https://drafts.csswg.org/css-animations-2/#typedef-timeline-name>
/// <https://drafts.csswg.org/css-animations/#typedef-keyframes-name>
///
/// 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 {
/// <custom-ident>
Ident(CustomIdent),
/// <string>
QuotedString(Atom),
}
pub struct TimelineOrKeyframesName(Atom);
impl TimelineOrKeyframesName {
/// <https://drafts.csswg.org/css-animations/#dom-csskeyframesrule-name>
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 <string>, but currently Gecko
// stores both of <custom-ident> and <string> 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<Self, ParseError<'i>> {
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<W>(&self, dest: &mut CssWriter<W>, 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 <timeline-name>.
#[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<H>(&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<Self, ParseError<'i>> {
Ok(Self(TimelineOrKeyframesName::parse(input, &["none", "auto"])?))
}
}
impl Parse for TimelineOrKeyframesName {
fn parse<'i, 't>(
_context: &ParserContext,
input: &mut Parser<'i, 't>,
) -> Result<Self, ParseError<'i>> {
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<W>(&self, dest: &mut CssWriter<W>) -> 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 <timeline-name>.
pub type TimelineName = TimelineOrKeyframesName;
/// The typedef of <keyframes-name>.
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<Self, ParseError<'i>> {
Ok(Self(TimelineOrKeyframesName::parse(input, &["none"])?))
}
}
impl ToCss for KeyframesName {
fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
where
W: Write,
{
self.0.to_css(dest, &["none"])
}
}

View file

@ -679,33 +679,30 @@ impl AnimationIterationCount {
PartialEq,
SpecifiedValueInfo,
ToComputedValue,
ToCss,
ToResolvedValue,
ToShmem,
)]
#[value_info(other_values = "none")]
pub struct AnimationName(pub Option<KeyframesName>);
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<W>(&self, dest: &mut CssWriter<W>) -> 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<Self, ParseError<'i>> {
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<T: Default + PartialEq>(value: &T) -> bool {
pub enum AnimationTimeline {
/// Use default timeline. The animations 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<Self, ParseError<'i>> {
// 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