Fix serialization of shorthands pending var() substitution.

components/script/dom/cssstyledeclaration.rs

@@ -16,8 +16,8 @@ use std::ascii::AsciiExt;
 use std::borrow::ToOwned;
 use std::cell::Ref;
 use string_cache::Atom;
-use style::properties::PropertyDeclaration;
+use style::properties::{PropertyDeclaration, Shorthand};
-use style::properties::{is_supported_property, longhands_from_shorthand, parse_one_declaration};
+use style::properties::{is_supported_property, parse_one_declaration};
 use util::str::{DOMString, str_join};
 
 // http://dev.w3.org/csswg/cssom/#the-cssstyledeclaration-interface
@@ -48,9 +48,27 @@ macro_rules! css_properties(
     );
 );
 
-fn serialize_list(list: &[Ref<PropertyDeclaration>]) -> DOMString {
+fn serialize_shorthand(shorthand: Shorthand, declarations: &[Ref<PropertyDeclaration>])
-    let str_iter = list.iter().map(|d| d.value());
+                       -> String {
-    DOMString(str_join(str_iter, " "))
+    // https://drafts.csswg.org/css-variables/#variables-in-shorthands
+    if let Some(css) = declarations[0].with_variables_from_shorthand(shorthand) {
+        if declarations[1..].iter()
+                            .all(|d| d.with_variables_from_shorthand(shorthand) == Some(css)) {
+            css.to_owned()
+        } else {
+            String::new()
+        }
+    } else {
+        if declarations.iter().any(|d| d.with_variables()) {
+            String::new()
+        } else {
+            let str_iter = declarations.iter().map(|d| d.value());
+            // FIXME: this needs property-specific code, which probably should be in style/
+            // "as appropriate according to the grammar of shorthand "
+            // https://drafts.csswg.org/cssom/#serialize-a-css-value
+            str_join(str_iter, " ")
+        }
+    }
 }
 
 impl CSSStyleDeclaration {
@@ -130,13 +148,12 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         }
 
         // Step 2
-        let longhand_properties = longhands_from_shorthand(&property);
+        if let Some(shorthand) = Shorthand::from_name(&property) {
-        if let Some(longhand_properties) = longhand_properties {
             // Step 2.1
             let mut list = vec!();
 
             // Step 2.2
-            for longhand in &*longhand_properties {
+            for longhand in shorthand.longhands() {
                 // Step 2.2.1
                 let declaration = owner.get_inline_style_declaration(&Atom::from_slice(&longhand));
 
@@ -148,7 +165,7 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
             }
 
             // Step 2.3
-            return serialize_list(&list);
+            return DOMString(serialize_shorthand(shorthand, &list));
         }
 
         // Step 3 & 4
@@ -166,12 +183,11 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         let property = Atom::from_slice(&property);
 
         // Step 2
-        let longhand_properties = longhands_from_shorthand(&property);
+        if let Some(shorthand) = Shorthand::from_name(&property) {
-        if let Some(longhand_properties) = longhand_properties {
             // Step 2.1 & 2.2 & 2.3
-            if longhand_properties.iter()
+            if shorthand.longhands().iter()
-                                  .map(|&longhand| self.GetPropertyPriority(DOMString(longhand.to_owned())))
+                                    .map(|&longhand| self.GetPropertyPriority(DOMString(longhand.to_owned())))
-                                  .all(|priority| priority == "important") {
+                                    .all(|priority| priority == "important") {
 
                 return DOMString("important".to_owned());
             }
@@ -261,8 +277,10 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         let element = self.owner.upcast::<Element>();
 
         // Step 5 & 6
-        match longhands_from_shorthand(&property) {
+        match Shorthand::from_name(&property) {
-            Some(properties) => element.set_inline_style_property_priority(properties, priority),
+            Some(shorthand) => {
+                element.set_inline_style_property_priority(shorthand.longhands(), priority)
+            }
             None => element.set_inline_style_property_priority(&[&*property], priority)
         }
 
@@ -292,10 +310,10 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
 
         let elem = self.owner.upcast::<Element>();
 
-        match longhands_from_shorthand(&property) {
+        match Shorthand::from_name(&property) {
             // Step 4
-            Some(longhands) => {
+            Some(shorthand) => {
-                for longhand in &*longhands {
+                for longhand in shorthand.longhands() {
                     elem.remove_inline_style_property(longhand)
                 }
             }

components/style/properties.mako.rs

@@ -217,7 +217,7 @@ pub mod longhands {
                                     css: css.into_owned(),
                                     first_token_type: first_token_type,
                                     base_url: context.base_url.clone(),
-                                    from_shorthand: Shorthand::None,
+                                    from_shorthand: None,
                                 })
                             }
                             specified
@@ -4922,7 +4922,7 @@ pub mod shorthands {
                                 css: css.clone().into_owned(),
                                 first_token_type: first_token_type,
                                 base_url: context.base_url.clone(),
-                                from_shorthand: Shorthand::${shorthand.camel_case},
+                                from_shorthand: Some(Shorthand::${shorthand.camel_case}),
                             }
                         ));
                     % endfor
@@ -5644,12 +5644,12 @@ mod property_bit_field {
                             ::stylesheets::Origin::Author, base_url);
                         Parser::new(&css).parse_entirely(|input| {
                             match from_shorthand {
-                                Shorthand::None => {
+                                None => {
                                     longhands::${property.ident}::parse_specified(&context, input)
                                 }
                                 % for shorthand in SHORTHANDS:
                                     % if property in shorthand.sub_properties:
-                                        Shorthand::${shorthand.camel_case} => {
+                                        Some(Shorthand::${shorthand.camel_case}) => {
                                             shorthands::${shorthand.ident}::parse_value(&context, input)
                                             .map(|result| match result.${property.ident} {
                                                 Some(value) => DeclaredValue::Value(value),
@@ -5813,12 +5813,39 @@ impl CSSWideKeyword {
 
 #[derive(Clone, Copy, Eq, PartialEq, Debug)]
 pub enum Shorthand {
-    None,
     % for property in SHORTHANDS:
         ${property.camel_case},
     % endfor
 }
 
+impl Shorthand {
+    pub fn from_name(name: &str) -> Option<Shorthand> {
+        match_ignore_ascii_case! { name,
+            % for property in SHORTHANDS[:-1]:
+                "${property.name}" => Some(Shorthand::${property.camel_case}),
+            % endfor
+            % for property in SHORTHANDS[-1:]:
+                "${property.name}" => Some(Shorthand::${property.camel_case})
+            % endfor
+            _ => None
+        }
+    }
+
+    pub fn longhands(&self) -> &'static [&'static str] {
+        % for property in SHORTHANDS:
+            static ${property.ident.upper()}: &'static [&'static str] = &[
+                % for sub in property.sub_properties:
+                    "${sub.name}",
+                % endfor
+            ];
+        % endfor
+        match *self {
+            % for property in SHORTHANDS:
+                Shorthand::${property.camel_case} => ${property.ident.upper()},
+            % endfor
+        }
+    }
+}
 
 #[derive(Clone, PartialEq, Eq, Debug)]
 pub enum DeclaredValue<T> {
@@ -5827,7 +5854,7 @@ pub enum DeclaredValue<T> {
         css: String,
         first_token_type: TokenSerializationType,
         base_url: Url,
-        from_shorthand: Shorthand
+        from_shorthand: Option<Shorthand>,
     },
     Initial,
     Inherit,
@@ -5840,7 +5867,7 @@ impl<T: ToCss> ToCss for DeclaredValue<T> {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
         match *self {
             DeclaredValue::Value(ref inner) => inner.to_css(dest),
-            DeclaredValue::WithVariables { ref css, from_shorthand: Shorthand::None, .. } => {
+            DeclaredValue::WithVariables { ref css, from_shorthand: None, .. } => {
                 dest.write_str(css)
             }
             // https://drafts.csswg.org/css-variables/#variables-in-shorthands
@@ -5930,6 +5957,41 @@ impl PropertyDeclaration {
         }
     }
 
+    /// If this is a pending-substitution value from the given shorthand, return that value
+    // Extra space here because < seems to be removed by Mako when immediately followed by &.
+    //                                                                          ↓
+    pub fn with_variables_from_shorthand(&self, shorthand: Shorthand) -> Option< &str> {
+        match *self {
+            % for property in LONGHANDS:
+                PropertyDeclaration::${property.camel_case}(ref value) => match *value {
+                    DeclaredValue::WithVariables { ref css, from_shorthand: Some(s), .. }
+                    if s == shorthand => {
+                        Some(&**css)
+                    }
+                    _ => None
+                },
+            % endfor
+            PropertyDeclaration::Custom(..) => None,
+        }
+    }
+
+    /// Return whether this is a pending-substitution value.
+    /// https://drafts.csswg.org/css-variables/#variables-in-shorthands
+    pub fn with_variables(&self) -> bool {
+        match *self {
+            % for property in LONGHANDS:
+                PropertyDeclaration::${property.camel_case}(ref value) => match *value {
+                    DeclaredValue::WithVariables { .. } => true,
+                    _ => false,
+                },
+            % endfor
+            PropertyDeclaration::Custom(_, ref value) => match *value {
+                DeclaredValue::WithVariables { .. } => true,
+                _ => false,
+            }
+        }
+    }
+
     pub fn matches(&self, name: &str) -> bool {
         match *self {
             % for property in LONGHANDS:
@@ -6912,27 +6974,6 @@ macro_rules! longhand_properties_idents {
     }
 }
 
-// Extra space here because < seems to be removed by Mako when immediately followed by &.
-//                                                         ↓
-pub fn longhands_from_shorthand(shorthand: &str) -> Option< &'static [&'static str]> {
-    % for property in SHORTHANDS:
-        static ${property.ident.upper()}: &'static [&'static str] = &[
-            % for sub in property.sub_properties:
-                "${sub.name}",
-            % endfor
-        ];
-    % endfor
-    match_ignore_ascii_case!{ shorthand,
-        % for property in SHORTHANDS[:-1]:
-            "${property.name}" => Some(${property.ident.upper()}),
-        % endfor
-        % for property in SHORTHANDS[-1:]:
-            "${property.name}" => Some(${property.ident.upper()})
-        % endfor
-        _ => None
-    }
-}
-
 /// Corresponds to the fields in `gfx::font_template::FontTemplateDescriptor`.
 fn compute_font_hash(font: &mut style_structs::Font) {
     let mut hasher: FnvHasher = Default::default();