Bug 1371518 - Only include shorthands with at least one animatable component in TransitionProperty; r?hiro draft
authorBrian Birtles <birtles@gmail.com>
Wed, 14 Jun 2017 14:34:31 +0900
changeset 593852 2ec206dda31926dd66ee5930a09dd5cf5140ea5d
parent 593851 faf5d7183ac77f3087afe5eff3d8551d3d2ed372
child 593853 63b47754692cd1ec8251a1497ed442f6cc5d3d7e
push id63840
push userbbirtles@mozilla.com
push dateWed, 14 Jun 2017 07:29:38 +0000
reviewershiro
bugs1371518
milestone56.0a1
Bug 1371518 - Only include shorthands with at least one animatable component in TransitionProperty; r?hiro This allows simplifying the code somewhat and means we can ignore unanimated shorthands a little sooner. Furthermore, it removes the odd inconsistency where TransitionProperty only included animatable longhands but allowed all shorthands regardless of whether or not they were animatable. MozReview-Commit-ID: Ki9XegCWN3y
servo/components/style/properties/helpers/animated_properties.mako.rs
--- a/servo/components/style/properties/helpers/animated_properties.mako.rs
+++ b/servo/components/style/properties/helpers/animated_properties.mako.rs
@@ -41,37 +41,31 @@ use values::computed::{Angle, LengthOrPe
 use values::computed::{BorderCornerRadius, ClipRect};
 use values::computed::{CalcLengthOrPercentage, Color, Context, ComputedValueAsSpecified};
 use values::computed::{LengthOrPercentage, MaxLength, MozLength, Shadow, ToComputedValue};
 use values::generics::{SVGPaint, SVGPaintKind};
 use values::generics::border::BorderCornerRadius as GenericBorderCornerRadius;
 use values::generics::position as generic_position;
 
 
-/// A given transition property, that is either `All`, or an animatable
-/// property.
+/// A given transition property, that is either `All`, an animatable longhand property,
+/// a shorthand with at least one animatable longhand component, or an unsupported property.
 // NB: This needs to be here because it needs all the longhands generated
 // beforehand.
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 pub enum TransitionProperty {
     /// All, any animatable property changing should generate a transition.
     All,
-    % for prop in data.longhands:
+    % for prop in data.longhands + data.shorthands_except_all():
         % if prop.animatable:
             /// ${prop.name}
             ${prop.camel_case},
         % endif
     % endfor
-    // Shorthand properties may or may not contain any animatable property. Either should still be
-    // parsed properly.
-    % for prop in data.shorthands_except_all():
-        /// ${prop.name}
-        ${prop.camel_case},
-    % endfor
     /// Unrecognized property which could be any non-animatable, custom property, or
     /// unknown property.
     Unsupported(Atom)
 }
 
 no_viewport_percentage!(TransitionProperty);
 
 impl ComputedValueAsSpecified for TransitionProperty {}
@@ -99,24 +93,21 @@ impl TransitionProperty {
         false
     }
 
     /// Parse a transition-property value.
     pub fn parse<'i, 't>(input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
         let ident = try!(input.expect_ident());
         (match_ignore_ascii_case! { &ident,
             "all" => Ok(TransitionProperty::All),
-            % for prop in data.longhands:
+            % for prop in data.longhands + data.shorthands_except_all():
                 % if prop.animatable:
                     "${prop.name}" => Ok(TransitionProperty::${prop.camel_case}),
                 % endif
             % endfor
-            % for prop in data.shorthands_except_all():
-                "${prop.name}" => Ok(TransitionProperty::${prop.camel_case}),
-            % endfor
             "none" => Err(()),
             _ => {
                 match CSSWideKeyword::from_ident(&ident) {
                     Some(_) => Err(()),
                     None => Ok(TransitionProperty::Unsupported((&*ident).into()))
                 }
             }
         }).map_err(|()| SelectorParseError::UnexpectedIdent(ident.into()).into())
@@ -159,37 +150,43 @@ impl TransitionProperty {
             % endfor
             _ => false
         }
     }
 
     /// Return animatable longhands of this shorthand TransitionProperty, except for "all".
     pub fn longhands(&self) -> &'static [TransitionProperty] {
         % for prop in data.shorthands_except_all():
-            static ${prop.ident.upper()}: &'static [TransitionProperty] = &[
-                % for sub in prop.sub_properties:
-                    % if sub.animatable:
-                        TransitionProperty::${sub.camel_case},
-                    % endif
-                % endfor
-            ];
+            % if prop.animatable:
+                static ${prop.ident.upper()}: &'static [TransitionProperty] = &[
+                    % for sub in prop.sub_properties:
+                        % if sub.animatable:
+                            TransitionProperty::${sub.camel_case},
+                        % endif
+                    % endfor
+                ];
+            % endif
         % endfor
         match *self {
             % for prop in data.shorthands_except_all():
-                TransitionProperty::${prop.camel_case} => ${prop.ident.upper()},
+                % if prop.animatable:
+                    TransitionProperty::${prop.camel_case} => ${prop.ident.upper()},
+                % endif
             % endfor
             _ => panic!("Not allowed to call longhands() for this TransitionProperty")
         }
     }
 
     /// Returns true if this TransitionProperty is a shorthand.
     pub fn is_shorthand(&self) -> bool {
         match *self {
             % for prop in data.shorthands_except_all():
-                TransitionProperty::${prop.camel_case} => true,
+                % if prop.animatable:
+                    TransitionProperty::${prop.camel_case} => true,
+                % endif
             % endfor
             _ => false
         }
     }
 }
 
 /// Returns true if this nsCSSPropertyID is one of the animatable properties.
 #[cfg(feature = "gecko")]
@@ -205,74 +202,63 @@ pub fn nscsspropertyid_is_animatable(pro
 }
 
 impl ToCss for TransitionProperty {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result
         where W: fmt::Write,
     {
         match *self {
             TransitionProperty::All => dest.write_str("all"),
-            % for prop in data.longhands:
+            % for prop in data.longhands + data.shorthands_except_all():
                 % if prop.animatable:
                     TransitionProperty::${prop.camel_case} => dest.write_str("${prop.name}"),
                 % endif
             % endfor
-            % 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),
         }
     }
 }
 
 /// Convert to nsCSSPropertyID.
 #[cfg(feature = "gecko")]
 #[allow(non_upper_case_globals)]
 impl<'a> From< &'a TransitionProperty> for nsCSSPropertyID {
     fn from(transition_property: &'a TransitionProperty) -> nsCSSPropertyID {
         match *transition_property {
-            % for prop in data.longhands:
+            % for prop in data.longhands + data.shorthands_except_all():
                 % if prop.animatable:
                     TransitionProperty::${prop.camel_case}
                         => ${helpers.to_nscsspropertyid(prop.ident)},
                 % endif
             % endfor
-            % for prop in data.shorthands_except_all():
-                TransitionProperty::${prop.camel_case}
-                    => ${helpers.to_nscsspropertyid(prop.ident)},
-            % endfor
             TransitionProperty::All => nsCSSPropertyID::eCSSPropertyExtra_all_properties,
             _ => panic!("Unconvertable Servo transition property: {:?}", transition_property),
         }
     }
 }
 
 /// Convert nsCSSPropertyID to TransitionProperty
 #[cfg(feature = "gecko")]
 #[allow(non_upper_case_globals)]
 impl From<nsCSSPropertyID> for TransitionProperty {
     fn from(property: nsCSSPropertyID) -> TransitionProperty {
         match property {
-            % for prop in data.longhands:
+            % for prop in data.longhands + data.shorthands_except_all():
                 % 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():
-                ${helpers.to_nscsspropertyid(prop.ident)}
-                    => TransitionProperty::${prop.camel_case},
-            % endfor
             nsCSSPropertyID::eCSSPropertyExtra_all_properties => TransitionProperty::All,
             _ => panic!("Unconvertable nsCSSPropertyID: {:?}", property),
         }
     }
 }
 
 /// Convert to PropertyDeclarationId.
 #[cfg(feature = "gecko")]