Use CustomIdent for animation-name and @keyframes

components/script/dom/csskeyframesrule.rs

@@ -16,11 +16,11 @@ use dom::cssrulelist::{CSSRuleList, RulesSource};
 use dom::cssstylesheet::CSSStyleSheet;
 use dom::window::Window;
 use dom_struct::dom_struct;
-use servo_atoms::Atom;
 use std::sync::Arc;
 use style::keyframes::{Keyframe, KeyframeSelector};
 use style::shared_lock::{Locked, ToCssWithGuard};
 use style::stylesheets::KeyframesRule;
+use style::values::CustomIdent;
 
 #[dom_struct]
 pub struct CSSKeyframesRule {
@@ -107,7 +107,7 @@ impl CSSKeyframesRuleMethods for CSSKeyframesRule {
     // https://drafts.csswg.org/css-animations/#dom-csskeyframesrule-name
     fn Name(&self) -> DOMString {
         let guard = self.cssrule.shared_lock().read();
-        DOMString::from(&*self.keyframesrule.read_with(&guard).name)
+        DOMString::from(&*self.keyframesrule.read_with(&guard).name.0)
     }
 
     // https://drafts.csswg.org/css-animations/#dom-csskeyframesrule-name
@@ -115,15 +115,9 @@ impl CSSKeyframesRuleMethods for CSSKeyframesRule {
         // https://github.com/w3c/csswg-drafts/issues/801
         // Setting this property to a CSS-wide keyword or `none` will
         // throw a Syntax Error.
-        match_ignore_ascii_case! { &value,
+        let name = CustomIdent::from_ident(value.into(), &["none"]).map_err(|()| Error::Syntax)?;
-            "initial" => return Err(Error::Syntax),
-            "inherit" => return Err(Error::Syntax),
-            "unset" => return Err(Error::Syntax),
-            "none" => return Err(Error::Syntax),
-            _ => ()
-        }
         let mut guard = self.cssrule.shared_lock().write();
-        self.keyframesrule.write_with(&mut guard).name = Atom::from(value);
+        self.keyframesrule.write_with(&mut guard).name = name;
         Ok(())
     }
 }

components/style/animation.rs

@@ -470,8 +470,8 @@ pub fn maybe_start_animations(context: &SharedStyleContext,
             continue
         }
 
-        if let Some(ref anim) = context.stylist.animations().get(&name.0) {
+        if let Some(ref anim) = context.stylist.animations().get(&name.0 .0) {
-            debug!("maybe_start_animations: animation {} found", name);
+            debug!("maybe_start_animations: animation {} found", name.0 .0);
 
             // If this animation doesn't have any keyframe, we can just continue
             // without submitting it to the compositor, since both the first and
@@ -506,7 +506,7 @@ pub fn maybe_start_animations(context: &SharedStyleContext,
 
 
             new_animations_sender
-                .send(Animation::Keyframes(node, name.0.clone(), KeyframesAnimationState {
+                .send(Animation::Keyframes(node, name.0 .0.clone(), KeyframesAnimationState {
                     started_at: animation_start,
                     duration: duration as f64,
                     delay: delay as f64,
@@ -586,7 +586,7 @@ pub fn update_style_for_animation(context: &SharedStyleContext,
 
             let maybe_index = style.get_box()
                                    .animation_name_iter()
-                                   .position(|animation_name| *name == animation_name.0);
+                                   .position(|animation_name| *name == animation_name.0 .0);
 
             let index = match maybe_index {
                 Some(index) => index,

components/style/matching.rs

@@ -560,7 +560,7 @@ trait PrivateMatchMethods: TElement {
                                pseudo: Option<&PseudoElement>) -> bool {
         let ref new_box_style = new_values.get_box();
         let has_new_animation_style = new_box_style.animation_name_count() >= 1 &&
-                                      new_box_style.animation_name_at(0).0.len() != 0;
+                                      new_box_style.animation_name_at(0).0 .0.len() != 0;
         let has_animations = self.has_css_animations(pseudo);
 
         old_values.as_ref().map_or(has_new_animation_style, |ref old| {

components/style/properties/gecko.mako.rs

@@ -62,7 +62,7 @@ use std::ptr;
 use std::sync::Arc;
 use std::cmp;
 use values::computed::ToComputedValue;
-use values::{Either, Auto};
+use values::{Either, Auto, CustomIdent};
 use computed_values::border_style;
 
 pub mod style_structs {
@@ -2205,7 +2205,7 @@ fn static_assert() {
         self.gecko.mAnimationNameCount = v.0.len() as u32;
         for (servo, gecko) in v.0.into_iter().zip(self.gecko.mAnimations.iter_mut()) {
             // TODO This is inefficient. We should fix this in bug 1329169.
-            gecko.mName.assign_utf8(&nsCString::from(servo.0.to_string()));
+            gecko.mName.assign_utf8(&nsCString::from(servo.0 .0.to_string()));
         }
     }
     pub fn animation_name_at(&self, index: usize)
@@ -2213,7 +2213,8 @@ fn static_assert() {
         use Atom;
         use properties::longhands::animation_name::single_value::SpecifiedValue as AnimationName;
         // XXX: Is there any effective ways?
-        AnimationName(Atom::from(String::from_utf16_lossy(&self.gecko.mAnimations[index].mName[..])))
+        AnimationName(CustomIdent(Atom::from(
+            String::from_utf16_lossy(&self.gecko.mAnimations[index].mName[..]))))
     }
     pub fn copy_animation_name_from(&mut self, other: &Self) {
         unsafe { self.gecko.mAnimations.ensure_len(other.gecko.mAnimations.len()) };

components/style/properties/longhand/box.mako.rs

@@ -796,7 +796,7 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
     use std::ops::Deref;
     use style_traits::ToCss;
     use values::computed::ComputedValueAsSpecified;
-    use values::HasViewportPercentage;
+    use values::{HasViewportPercentage, CustomIdent};
 
     pub mod computed_value {
         pub use super::SpecifiedValue as T;
@@ -804,7 +804,7 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
 
     #[derive(Clone, Debug, Hash, Eq, PartialEq)]
     #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-    pub struct SpecifiedValue(pub Atom);
+    pub struct SpecifiedValue(pub CustomIdent);
 
     #[inline]
     pub fn get_initial_value() -> computed_value::T {
@@ -813,21 +813,21 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
 
     #[inline]
     pub fn get_initial_specified_value() -> SpecifiedValue {
-        SpecifiedValue(atom!(""))
+        SpecifiedValue(CustomIdent(atom!("")))
     }
 
     impl fmt::Display for SpecifiedValue {
         fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-            self.0.fmt(f)
+            self.0 .0.fmt(f)
         }
     }
 
     impl ToCss for SpecifiedValue {
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            if self.0 == atom!("") {
+            if self.0 .0 == atom!("") {
                 dest.write_str("none")
             } else {
-                dest.write_str(&*self.0.to_string())
+                self.0.to_css(dest)
             }
         }
     }
@@ -835,23 +835,19 @@ ${helpers.single_keyword("overflow-x", "visible hidden scroll auto",
     impl Parse for SpecifiedValue {
         fn parse(_context: &ParserContext, input: &mut ::cssparser::Parser) -> Result<Self, ()> {
             use cssparser::Token;
-            use properties::CSSWideKeyword;
             use std::ascii::AsciiExt;
+            use values::CustomIdent;
 
             let atom = match input.next() {
-                Ok(Token::Ident(ref value)) => {
+                Ok(Token::Ident(value)) => {
-                    if CSSWideKeyword::from_ident(value).is_some() {
+                    if value.eq_ignore_ascii_case("none") {
-                        // We allow any ident for the animation-name except one
-                        // of the CSS-wide keywords.
-                        return Err(());
-                    } else if value.eq_ignore_ascii_case("none") {
                         // FIXME We may want to support `@keyframes ""` at some point.
-                        atom!("")
+                        CustomIdent(atom!(""))
                     } else {
-                        Atom::from(&**value)
+                        CustomIdent::from_ident(value, &[])?
                     }
                 }
-                Ok(Token::QuotedString(value)) => Atom::from(&*value),
+                Ok(Token::QuotedString(value)) => CustomIdent(Atom::from(value)),
                 _ => return Err(()),
             };
             Ok(SpecifiedValue(atom))

components/style/properties/properties.mako.rs

@@ -1617,7 +1617,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 != atom!(""))
+                self.animation_name_iter().any(|name| name.0 .0 != atom!(""))
             }
 
             /// Returns whether there are any transitions specified.

components/style/stylesheets.rs

@@ -40,6 +40,7 @@ use str::starts_with_ignore_ascii_case;
 use style_traits::ToCss;
 use stylist::FnvHashMap;
 use supports::SupportsCondition;
+use values::CustomIdent;
 use values::specified::url::SpecifiedUrl;
 use viewport::ViewportRule;
 
@@ -513,7 +514,7 @@ impl ToCssWithGuard for ImportRule {
 #[derive(Debug)]
 pub struct KeyframesRule {
     /// The name of the current animation.
-    pub name: Atom,
+    pub name: CustomIdent,
     /// The keyframes specified for this CSS rule.
     pub keyframes: Vec<Arc<Locked<Keyframe>>>,
     /// Vendor prefix type the @keyframes has.
@@ -525,7 +526,7 @@ impl ToCssWithGuard for KeyframesRule {
     fn to_css<W>(&self, guard: &SharedRwLockReadGuard, dest: &mut W) -> fmt::Result
     where W: fmt::Write {
         try!(dest.write_str("@keyframes "));
-        try!(dest.write_str(&*self.name.to_string()));
+        try!(self.name.to_css(dest));
         try!(dest.write_str(" { "));
         let iter = self.keyframes.iter();
         let mut first = true;
@@ -922,7 +923,7 @@ enum AtRulePrelude {
     /// A @viewport rule prelude.
     Viewport,
     /// A @keyframes rule, with its animation name and vendor prefix if exists.
-    Keyframes(Atom, Option<VendorPrefix>),
+    Keyframes(CustomIdent, Option<VendorPrefix>),
     /// A @page rule prelude.
     Page,
 }
@@ -1123,12 +1124,12 @@ impl<'a, 'b> AtRuleParser for NestedRuleParser<'a, 'b> {
                     return Err(())
                 }
                 let name = match input.next() {
-                    Ok(Token::Ident(ref value)) if value != "none" => Atom::from(&**value),
+                    Ok(Token::Ident(value)) => CustomIdent::from_ident(value, &["none"])?,
-                    Ok(Token::QuotedString(value)) => Atom::from(&*value),
+                    Ok(Token::QuotedString(value)) => CustomIdent(Atom::from(value)),
                     _ => return Err(())
                 };
 
-                Ok(AtRuleType::WithBlock(AtRulePrelude::Keyframes(Atom::from(name), prefix)))
+                Ok(AtRuleType::WithBlock(AtRulePrelude::Keyframes(name, prefix)))
             },
             "page" => {
                 if cfg!(feature = "gecko") {

components/style/stylist.rs

@@ -349,13 +349,13 @@ impl Stylist {
 
                     // Don't let a prefixed keyframes animation override a non-prefixed one.
                     let needs_insertion = keyframes_rule.vendor_prefix.is_none() ||
-                        self.animations.get(&keyframes_rule.name).map_or(true, |rule|
+                        self.animations.get(&keyframes_rule.name.0).map_or(true, |rule|
                             rule.vendor_prefix.is_some());
                     if needs_insertion {
                         let animation = KeyframesAnimation::from_keyframes(
                             &keyframes_rule.keyframes, keyframes_rule.vendor_prefix.clone(), guard);
                         debug!("Found valid keyframe animation: {:?}", animation);
-                        self.animations.insert(keyframes_rule.name.clone(), animation);
+                        self.animations.insert(keyframes_rule.name.0.clone(), animation);
                     }
                 }
                 CssRule::FontFace(ref rule) => {