Make animation-name parse none draft
authorXidorn Quan <me@upsuper.org>
Mon, 06 Mar 2017 21:47:05 +1100
changeset 494020 8a6c0ffbe4daa67ebbd7e40000678ff317a75ec1
parent 493803 b78daeae90627fe81e38b47954ecae48b597c87a
child 494021 bd61684f07f9cba13a9e31617c5c8cf54628d969
push id47899
push userxquan@mozilla.com
push dateMon, 06 Mar 2017 10:53:41 +0000
milestone54.0a1
Make animation-name parse none MozReview-Commit-ID: DM0Z1WUh5Tp
servo/components/style/matching.rs
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/longhand/box.mako.rs
servo/components/style/properties/properties.mako.rs
servo/tests/unit/style/lib.rs
servo/tests/unit/style/parsing/animation.rs
--- a/servo/components/style/matching.rs
+++ b/servo/components/style/matching.rs
@@ -393,17 +393,17 @@ impl<E: TElement> StyleSharingCandidateC
         }
 
         let box_style = style.get_box();
         if box_style.transition_property_count() > 0 {
             debug!("Failing to insert to the cache: transitions");
             return;
         }
 
-        if box_style.animation_name_count() > 0 {
+        if box_style.specifies_animations() {
             debug!("Failing to insert to the cache: animations");
             return;
         }
 
         debug!("Inserting into cache: {:?} with parent {:?}",
                element, parent);
 
         self.cache.insert(StyleSharingCandidate {
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -1722,25 +1722,24 @@ fn static_assert() {
             result.push(servo);
             unsafe { cur = (&*cur).mNext };
         }
         computed_value::T(Some(result))
     }
 
     pub fn set_animation_name(&mut self, v: longhands::animation_name::computed_value::T) {
         use nsstring::nsCString;
+
+        debug_assert!(!v.0.is_empty());
         unsafe { self.gecko.mAnimations.ensure_len(v.0.len()) };
 
-        if v.0.len() > 0 {
-            self.gecko.mAnimationNameCount = v.0.len() as u32;
-            for (servo, gecko) in v.0.into_iter().zip(self.gecko.mAnimations.iter_mut()) {
-                gecko.mName.assign_utf8(&nsCString::from(servo.0.to_string()));
-            }
-        } else {
-            unsafe { self.gecko.mAnimations[0].mName.truncate(); }
+        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()));
         }
     }
     pub fn animation_name_at(&self, index: usize)
         -> longhands::animation_name::computed_value::SingleComputedValue {
         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[..])))
--- a/servo/components/style/properties/longhand/box.mako.rs
+++ b/servo/components/style/properties/longhand/box.mako.rs
@@ -771,17 +771,16 @@
                           extra_prefixes="moz webkit"
                           spec="https://drafts.csswg.org/css-transitions/#propdef-transition-delay">
     pub use properties::longhands::transition_duration::single_value::SpecifiedValue;
     pub use properties::longhands::transition_duration::single_value::computed_value;
     pub use properties::longhands::transition_duration::single_value::{get_initial_value, parse};
 </%helpers:vector_longhand>
 
 <%helpers:vector_longhand name="animation-name"
-                          allow_empty="True"
                           need_index="True"
                           animatable="False",
                           extra_prefixes="moz webkit"
                           allowed_in_keyframe_block="False"
                           spec="https://drafts.csswg.org/css-animations/#propdef-animation-name">
     use Atom;
     use std::fmt;
     use std::ops::Deref;
@@ -792,33 +791,52 @@
     pub mod computed_value {
         pub use super::SpecifiedValue as T;
     }
 
     #[derive(Clone, Debug, Hash, Eq, PartialEq)]
     #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
     pub struct SpecifiedValue(pub Atom);
 
+    #[inline]
+    pub fn get_initial_value() -> computed_value::T {
+        get_initial_specified_value()
+    }
+
+    #[inline]
+    pub fn get_initial_specified_value() -> SpecifiedValue {
+        SpecifiedValue(atom!(""))
+    }
+
     impl fmt::Display for SpecifiedValue {
         fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
             self.0.fmt(f)
         }
     }
 
     impl ToCss for SpecifiedValue {
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            dest.write_str(&*self.0.to_string())
+            if self.0 == atom!("") {
+                dest.write_str("none")
+            } else {
+                dest.write_str(&*self.0.to_string())
+            }
         }
     }
 
     impl Parse for SpecifiedValue {
         fn parse(_context: &ParserContext, input: &mut ::cssparser::Parser) -> Result<Self, ()> {
             use cssparser::Token;
             Ok(match input.next() {
-                Ok(Token::Ident(ref value)) if value != "none" => SpecifiedValue(Atom::from(&**value)),
+                Ok(Token::Ident(ref value)) => SpecifiedValue(if value == "none" {
+                    // FIXME We may want to support `@keyframes ""` at some point.
+                    atom!("")
+                } else {
+                    Atom::from(&**value)
+                }),
                 Ok(Token::QuotedString(value)) => SpecifiedValue(Atom::from(&*value)),
                 _ => return Err(()),
             })
         }
     }
     no_viewport_percentage!(SpecifiedValue);
 
     pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue,()> {
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -1290,16 +1290,24 @@ pub mod style_structs {
                 #[allow(non_snake_case)]
                 #[inline]
                 pub fn ${longhand.ident}_mod(&self, index: usize)
                     -> longhands::${longhand.ident}::computed_value::SingleComputedValue {
                     self.${longhand.ident}_at(index % self.${longhand.ident}_count())
                 }
             % endif
         % endfor
+
+        % if style_struct.name == "Box":
+            /// 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!(""))
+            }
+        % endif
     }
 
     % for longhand in style_struct.longhands:
         % if longhand.need_index:
             /// An iterator over the values of the ${longhand.name} properties.
             pub struct ${longhand.camel_case}Iter<'a> {
                 style_struct: &'a style_structs::${style_struct.name},
                 current: usize,
--- a/servo/tests/unit/style/lib.rs
+++ b/servo/tests/unit/style/lib.rs
@@ -9,17 +9,17 @@ extern crate app_units;
 extern crate cssparser;
 extern crate euclid;
 #[macro_use] extern crate html5ever_atoms;
 extern crate owning_ref;
 extern crate parking_lot;
 extern crate rayon;
 extern crate rustc_serialize;
 extern crate selectors;
-extern crate servo_atoms;
+#[macro_use] extern crate servo_atoms;
 extern crate servo_config;
 extern crate servo_url;
 extern crate style;
 extern crate style_traits;
 extern crate test;
 
 mod animated_properties;
 mod attr;
--- a/servo/tests/unit/style/parsing/animation.rs
+++ b/servo/tests/unit/style/parsing/animation.rs
@@ -1,21 +1,37 @@
 /* This Source Code Form is subject to the terms of the Mozilla Public
  * License, v. 2.0. If a copy of the MPL was not distributed with this
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use cssparser::Parser;
 use media_queries::CSSErrorReporterTest;
 use parsing::parse;
+use servo_atoms::Atom;
 use style::parser::{Parse, ParserContext};
 use style::properties::longhands::animation_iteration_count::single_value::computed_value::T as AnimationIterationCount;
+use style::properties::longhands::animation_name;
 use style::stylesheets::Origin;
 use style_traits::ToCss;
 
 #[test]
+fn test_animation_name() {
+    use self::animation_name::single_value::SpecifiedValue as SingleValue;
+    let other_name = Atom::from("other-name");
+    assert_eq!(parse_longhand!(animation_name, "none"),
+               animation_name::SpecifiedValue(vec![SingleValue(atom!(""))]));
+    assert_eq!(parse_longhand!(animation_name, "other-name, none, 'other-name', \"other-name\""),
+               animation_name::SpecifiedValue(
+                   vec![SingleValue(other_name.clone()),
+                        SingleValue(atom!("")),
+                        SingleValue(other_name.clone()),
+                        SingleValue(other_name.clone())]));
+}
+
+#[test]
 fn test_animation_iteration() {
     assert_roundtrip_with_context!(AnimationIterationCount::parse, "0", "0");
     assert_roundtrip_with_context!(AnimationIterationCount::parse, "0.1", "0.1");
     assert_roundtrip_with_context!(AnimationIterationCount::parse, "infinite", "infinite");
 
     // Negative numbers are invalid
     assert!(parse(AnimationIterationCount::parse, "-1").is_err());
 }