Bug 1357357 - Make the parser of transition-property match the spec.

1. We add a new arm to TransitionProperty, TransitionProperty::Unsupported,
   which contains an Atom, so it's better to remove the Copy trait from
   TransitionProperty.
2. TransitionProperty::Unsupported(Atom) represents any non-animatable, custom,
   or unrecognized property, and we use Atom to store the ident string for
   serialization.

components/style/properties/gecko.mako.rs

@@ -33,6 +33,7 @@ use gecko_bindings::bindings::Gecko_FontFamilyList_AppendNamed;
 use gecko_bindings::bindings::Gecko_FontFamilyList_Clear;
 use gecko_bindings::bindings::Gecko_SetCursorArrayLength;
 use gecko_bindings::bindings::Gecko_SetCursorImage;
+use gecko_bindings::bindings::Gecko_StyleTransition_SetUnsupportedProperty;
 use gecko_bindings::bindings::Gecko_NewCSSShadowArray;
 use gecko_bindings::bindings::Gecko_nsStyleFont_SetLang;
 use gecko_bindings::bindings::Gecko_nsStyleFont_CopyLangFrom;
@@ -53,6 +54,7 @@ use gecko::values::convert_rgba_to_nscolor;
 use gecko::values::GeckoStyleCoordConvertible;
 use gecko::values::round_border_to_device_pixels;
 use logical_geometry::WritingMode;
+use properties::animated_properties::TransitionProperty;
 use properties::longhands;
 use properties::{Importance, LonghandId};
 use properties::{PropertyDeclaration, PropertyDeclarationBlock, PropertyDeclarationId};
@@ -2149,7 +2151,12 @@ fn static_assert() {
             unsafe { self.gecko.mTransitions.ensure_len(v.0.len()) };
             self.gecko.mTransitionPropertyCount = v.0.len() as u32;
             for (servo, gecko) in v.0.into_iter().zip(self.gecko.mTransitions.iter_mut()) {
-                gecko.mProperty = servo.into();
+                match servo {
+                    TransitionProperty::Unsupported(ref atom) => unsafe {
+                        Gecko_StyleTransition_SetUnsupportedProperty(gecko, atom.as_ptr())
+                    },
+                    _ => gecko.mProperty = (&servo).into(),
+                }
             }
         } else {
             // In gecko |none| is represented by eCSSPropertyExtra_no_properties.
@@ -2172,7 +2179,22 @@ fn static_assert() {
 
     pub fn transition_property_at(&self, index: usize)
         -> longhands::transition_property::computed_value::SingleComputedValue {
-        self.gecko.mTransitions[index].mProperty.into()
+        use gecko_bindings::structs::nsCSSPropertyID::eCSSPropertyExtra_no_properties;
+        use gecko_bindings::structs::nsCSSPropertyID::eCSSPropertyExtra_variable;
+        use gecko_bindings::structs::nsCSSPropertyID::eCSSProperty_UNKNOWN;
+
+        let property = self.gecko.mTransitions[index].mProperty;
+        if property == eCSSProperty_UNKNOWN || property == eCSSPropertyExtra_variable {
+            let atom = self.gecko.mTransitions[index].mUnknownProperty.raw();
+            debug_assert!(!atom.is_null());
+            TransitionProperty::Unsupported(atom.into())
+        } else if property == eCSSPropertyExtra_no_properties {
+            // Actually, we don't expect TransitionProperty::Unsupported also represents "none",
+            // but if the caller wants to convert it, it is fine. Please use it carefully.
+            TransitionProperty::Unsupported(atom!("none"))
+        } else {
+            property.into()
+        }
     }
 
     pub fn transition_nscsspropertyid_at(&self, index: usize) -> nsCSSPropertyID {
@@ -2180,6 +2202,8 @@ fn static_assert() {
     }
 
     pub fn copy_transition_property_from(&mut self, other: &Self) {
+        use gecko_bindings::structs::nsCSSPropertyID::eCSSPropertyExtra_variable;
+        use gecko_bindings::structs::nsCSSPropertyID::eCSSProperty_UNKNOWN;
         unsafe { self.gecko.mTransitions.ensure_len(other.gecko.mTransitions.len()) };
 
         let count = other.gecko.mTransitionPropertyCount;
@@ -2187,6 +2211,12 @@ fn static_assert() {
 
         for (index, transition) in self.gecko.mTransitions.iter_mut().enumerate().take(count as usize) {
             transition.mProperty = other.gecko.mTransitions[index].mProperty;
+            if transition.mProperty == eCSSProperty_UNKNOWN ||
+               transition.mProperty == eCSSPropertyExtra_variable {
+                let atom = other.gecko.mTransitions[index].mUnknownProperty.raw();
+                debug_assert!(!atom.is_null());
+                unsafe { Gecko_StyleTransition_SetUnsupportedProperty(transition, atom) };
+            }
         }
     }
     ${impl_transition_count('property', 'Property')}

components/style/properties/helpers/animated_properties.mako.rs

@@ -7,9 +7,10 @@
 <% from data import SYSTEM_FONT_LONGHANDS %>
 
 use app_units::Au;
-use cssparser::{Color as CSSParserColor, Parser, RGBA};
+use cssparser::{Color as CSSParserColor, Parser, RGBA, serialize_identifier};
 use euclid::{Point2D, Size2D};
 #[cfg(feature = "gecko")] use gecko_bindings::structs::nsCSSPropertyID;
+#[cfg(feature = "gecko")] use gecko_string_cache::Atom;
 use properties::{CSSWideKeyword, PropertyDeclaration};
 use properties::longhands;
 use properties::longhands::background_size::computed_value::T as BackgroundSizeList;
@@ -25,6 +26,7 @@ use properties::longhands::transform::computed_value::T as TransformList;
 use properties::longhands::vertical_align::computed_value::T as VerticalAlign;
 use properties::longhands::visibility::computed_value::T as Visibility;
 #[cfg(feature = "gecko")] use properties::{PropertyDeclarationId, LonghandId};
+#[cfg(feature = "servo")] use servo_atoms::Atom;
 use std::cmp;
 #[cfg(feature = "gecko")] use std::collections::HashMap;
 use std::fmt;
@@ -45,7 +47,7 @@ use values::generics::position as generic_position;
 /// property.
 // NB: This needs to be here because it needs all the longhands generated
 // beforehand.
-#[derive(Copy, Clone, Debug, PartialEq, Eq, Hash)]
+#[derive(Clone, Debug, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum TransitionProperty {
     /// All, any animatable property changing should generate a transition.
@@ -62,24 +64,27 @@ pub enum TransitionProperty {
         /// ${prop.name}
         ${prop.camel_case},
     % endfor
+    /// Unrecognized property which could be any non-animatable, custom property, or
+    /// unknown property.
+    Unsupported(Atom)
 }
 
 impl TransitionProperty {
     /// Iterates over each longhand property.
-    pub fn each<F: FnMut(TransitionProperty) -> ()>(mut cb: F) {
+    pub fn each<F: FnMut(&TransitionProperty) -> ()>(mut cb: F) {
         % for prop in data.longhands:
             % if prop.animatable:
-                cb(TransitionProperty::${prop.camel_case});
+                cb(&TransitionProperty::${prop.camel_case});
             % endif
         % endfor
     }
 
     /// Iterates over every property that is not TransitionProperty::All, stopping and returning
     /// true when the provided callback returns true for the first time.
-    pub fn any<F: FnMut(TransitionProperty) -> bool>(mut cb: F) -> bool {
+    pub fn any<F: FnMut(&TransitionProperty) -> bool>(mut cb: F) -> bool {
         % for prop in data.longhands:
             % if prop.animatable:
-                if cb(TransitionProperty::${prop.camel_case}) {
+                if cb(&TransitionProperty::${prop.camel_case}) {
                     return true;
                 }
             % endif
@@ -89,7 +94,8 @@ impl TransitionProperty {
 
     /// Parse a transition-property value.
     pub fn parse(input: &mut Parser) -> Result<Self, ()> {
-        match_ignore_ascii_case! { &try!(input.expect_ident()),
+        let ident = try!(input.expect_ident());
+        match_ignore_ascii_case! { &ident,
             "all" => Ok(TransitionProperty::All),
             % for prop in data.longhands:
                 % if prop.animatable:
@@ -99,7 +105,13 @@ impl TransitionProperty {
             % for prop in data.shorthands_except_all():
                 "${prop.name}" => Ok(TransitionProperty::${prop.camel_case}),
             % endfor
-            _ => Err(())
+            "none" => Err(()),
+            _ => {
+                match CSSWideKeyword::from_ident(&ident) {
+                    Some(_) => Err(()),
+                    None => Ok(TransitionProperty::Unsupported((&*ident).into()))
+                }
+            }
         }
     }
 
@@ -199,6 +211,11 @@ impl ToCss for TransitionProperty {
             % for prop in data.shorthands_except_all():
                 TransitionProperty::${prop.camel_case} => dest.write_str("${prop.name}"),
             % endfor
+            #[cfg(feature = "gecko")]
+            TransitionProperty::Unsupported(ref atom) => serialize_identifier(&atom.to_string(),
+                                                                              dest),
+            #[cfg(feature = "servo")]
+            TransitionProperty::Unsupported(ref atom) => serialize_identifier(atom, dest),
         }
     }
 }
@@ -206,9 +223,9 @@ impl ToCss for TransitionProperty {
 /// Convert to nsCSSPropertyID.
 #[cfg(feature = "gecko")]
 #[allow(non_upper_case_globals)]
-impl From<TransitionProperty> for nsCSSPropertyID {
-    fn from(transition_property: TransitionProperty) -> nsCSSPropertyID {
-        match transition_property {
+impl<'a> From< &'a TransitionProperty> for nsCSSPropertyID {
+    fn from(transition_property: &'a TransitionProperty) -> nsCSSPropertyID {
+        match *transition_property {
             % for prop in data.longhands:
                 % if prop.animatable:
                     TransitionProperty::${prop.camel_case}
@@ -220,6 +237,7 @@ impl From<TransitionProperty> for nsCSSPropertyID {
                     => ${helpers.to_nscsspropertyid(prop.ident)},
             % endfor
             TransitionProperty::All => nsCSSPropertyID::eCSSPropertyExtra_all_properties,
+            _ => panic!("Unconvertable Servo transition property: {:?}", transition_property),
         }
     }
 }
@@ -234,6 +252,9 @@ impl From<nsCSSPropertyID> for TransitionProperty {
                 % if prop.animatable:
                     ${helpers.to_nscsspropertyid(prop.ident)}
                         => TransitionProperty::${prop.camel_case},
+                % else:
+                    ${helpers.to_nscsspropertyid(prop.ident)}
+                        => TransitionProperty::Unsupported(Atom::from("${prop.ident}")),
                 % endif
             % endfor
             % for prop in data.shorthands_except_all():
@@ -241,7 +262,7 @@ impl From<nsCSSPropertyID> for TransitionProperty {
                     => TransitionProperty::${prop.camel_case},
             % endfor
             nsCSSPropertyID::eCSSPropertyExtra_all_properties => TransitionProperty::All,
-            _ => panic!("Unsupported Servo transition property: {:?}", property),
+            _ => panic!("Unconvertable nsCSSPropertyID: {:?}", property),
         }
     }
 }
@@ -356,7 +377,7 @@ impl AnimatedProperty {
                     }
                 % endif
             % endfor
-            other => panic!("Can't use TransitionProperty::{:?} here", other),
+            ref other => panic!("Can't use TransitionProperty::{:?} here", other),
         }
     }
 }
@@ -544,7 +565,7 @@ impl AnimationValue {
                     }
                 % endif
             % endfor
-            other => panic!("Can't use TransitionProperty::{:?} here.", other),
+            ref other => panic!("Can't use TransitionProperty::{:?} here.", other),
         }
     }
 }

components/style/properties/properties.mako.rs

@@ -258,7 +258,7 @@ impl LonghandIdSet {
                     TransitionProperty::${prop.camel_case} => self.insert(LonghandId::${prop.camel_case}),
                 % endif
             % endfor
-            other => unreachable!("Tried to set TransitionProperty::{:?} in a PropertyBitfield", other),
+            ref other => unreachable!("Tried to set TransitionProperty::{:?} in a PropertyBitfield", other),
         }
     }
 
@@ -271,7 +271,7 @@ impl LonghandIdSet {
                     TransitionProperty::${prop.camel_case} => self.contains(LonghandId::${prop.camel_case}),
                 % endif
             % endfor
-            other => unreachable!("Tried to get TransitionProperty::{:?} in a PropertyBitfield", other),
+            ref other => unreachable!("Tried to get TransitionProperty::{:?} in a PropertyBitfield", other),
         }
     }
 }

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

@@ -98,10 +98,12 @@ macro_rules! try_parse_one {
             loop {
                 parsed += 1;
 
-                try_parse_one!(input, property, transition_property);
                 try_parse_one!(context, input, duration, transition_duration);
                 try_parse_one!(context, input, timing_function, transition_timing_function);
                 try_parse_one!(context, input, delay, transition_delay);
+                // Must check 'transition-property' after 'transition-timing-function' since
+                // 'transition-property' accepts any keyword.
+                try_parse_one!(input, property, transition_property);
 
                 parsed -= 1;
                 break