Auto merge of #11377 - servo:style-attr-ser-rebase, r=SimonSapin

Update style attributes on CSSStyleDeclaration changes

Rebase of #9410. Fixes #9410, fixes #9307.

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="35" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/11377)
<!-- Reviewable:end -->

components/script/dom/attr.rs

@@ -173,13 +173,18 @@ impl Attr {
     pub fn set_value(&self, mut value: AttrValue, owner: &Element) {
         assert!(Some(owner) == self.owner().r());
         owner.will_mutate_attr();
-        mem::swap(&mut *self.value.borrow_mut(), &mut value);
+        self.swap_value(&mut value);
         if self.identifier.namespace == ns!() {
             vtable_for(owner.upcast())
                 .attribute_mutated(self, AttributeMutation::Set(Some(&value)));
         }
     }
 
+    /// Used to swap the attribute's value without triggering mutation events
+    pub fn swap_value(&self, value: &mut AttrValue) {
+        mem::swap(&mut *self.value.borrow_mut(), value);
+    }
+
     pub fn identifier(&self) -> &AttrIdentifier {
         &self.identifier
     }

components/script/dom/cssstyledeclaration.rs

@@ -13,14 +13,13 @@ use dom::element::{Element, StylePriority};
 use dom::node::{Node, NodeDamage, window_from_node};
 use dom::window::Window;
 use std::ascii::AsciiExt;
-use std::borrow::ToOwned;
 use std::cell::Ref;
+use std::slice;
 use string_cache::Atom;
 use style::parser::ParserContextExtraData;
 use style::properties::{PropertyDeclaration, Shorthand};
 use style::properties::{is_supported_property, parse_one_declaration};
 use style::selector_impl::PseudoElement;
-use util::str::str_join;
 
 // http://dev.w3.org/csswg/cssom/#the-cssstyledeclaration-interface
 #[dom_struct]
@@ -50,29 +49,6 @@ macro_rules! css_properties(
     );
 );
 
-fn serialize_shorthand(shorthand: Shorthand, declarations: &[Ref<PropertyDeclaration>]) -> String {
-    // 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 {
     pub fn new_inherited(owner: &Element,
                          pseudo: Option<PseudoElement>,
@@ -172,7 +148,19 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
             }
 
             // Step 2.3
-            return DOMString::from(serialize_shorthand(shorthand, &list));
+            // Work around closures not being Clone
+            #[derive(Clone)]
+            struct Map<'a, 'b: 'a>(slice::Iter<'a, Ref<'b, PropertyDeclaration>>);
+            impl<'a, 'b> Iterator for Map<'a, 'b> {
+                type Item = &'a PropertyDeclaration;
+                fn next(&mut self) -> Option<Self::Item> {
+                    self.0.next().map(|r| &**r)
+                }
+            }
+
+            // TODO: important is hardcoded to false because method does not implement it yet
+            let serialized_value = shorthand.serialize_shorthand_value_to_string(Map(list.iter()), false);
+            return DOMString::from(serialized_value);
         }
 
         // Step 3 & 4
@@ -255,10 +243,8 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         let element = self.owner.upcast::<Element>();
 
         // Step 8
-        for decl in declarations {
+        // Step 9
-            // Step 9
+        element.update_inline_style(declarations, priority);
-            element.update_inline_style(decl, priority);
-        }
 
         let node = element.upcast::<Node>();
         node.dirty(NodeDamage::NodeStyleDamaged);
@@ -317,20 +303,20 @@ impl CSSStyleDeclarationMethods for CSSStyleDeclaration {
         // Step 3
         let value = self.GetPropertyValue(property.clone());
 
-        let elem = self.owner.upcast::<Element>();
+        let element = self.owner.upcast::<Element>();
 
         match Shorthand::from_name(&property) {
             // Step 4
             Some(shorthand) => {
                 for longhand in shorthand.longhands() {
-                    elem.remove_inline_style_property(longhand)
+                    element.remove_inline_style_property(longhand)
                 }
             }
             // Step 5
-            None => elem.remove_inline_style_property(&property),
+            None => element.remove_inline_style_property(&property),
         }
 
-        let node = elem.upcast::<Node>();
+        let node = element.upcast::<Node>();
         node.dirty(NodeDamage::NodeStyleDamaged);
 
         // Step 6

components/script/dom/element.rs

@@ -5,7 +5,7 @@
 //! Element nodes.
 
 use app_units::Au;
-use cssparser::Color;
+use cssparser::{Color, ToCss};
 use devtools_traits::AttrInfo;
 use dom::activation::Activatable;
 use dom::attr::AttrValue;
@@ -698,89 +698,145 @@ impl Element {
         }
     }
 
-    pub fn remove_inline_style_property(&self, property: &str) {
+    // this sync method is called upon modification of the style_attribute property,
-        let mut inline_declarations = self.style_attribute.borrow_mut();
+    // therefore, it should not trigger subsequent mutation events
-        if let &mut Some(ref mut declarations) = &mut *inline_declarations {
+    fn sync_property_with_attrs_style(&self) {
-            let index = declarations.normal
+        let style_str = if let &Some(ref declarations) = &*self.style_attribute().borrow() {
-                                    .iter()
+            declarations.to_css_string()
-                                    .position(|decl| decl.matches(property));
+        } else {
-            if let Some(index) = index {
+            String::new()
-                Arc::make_mut(&mut declarations.normal).remove(index);
+        };
-                return;
-            }
 
-            let index = declarations.important
+        let mut new_style = AttrValue::String(style_str);
-                                    .iter()
-                                    .position(|decl| decl.matches(property));
-            if let Some(index) = index {
-                Arc::make_mut(&mut declarations.important).remove(index);
-                return;
-            }
-        }
-    }
 
-    pub fn update_inline_style(&self,
+        if let Some(style_attr) = self.attrs.borrow().iter().find(|a| a.name() == &atom!("style")) {
-                               property_decl: PropertyDeclaration,
+            style_attr.swap_value(&mut new_style);
-                               style_priority: StylePriority) {
-        let mut inline_declarations = self.style_attribute().borrow_mut();
-        if let &mut Some(ref mut declarations) = &mut *inline_declarations {
-            let existing_declarations = if style_priority == StylePriority::Important {
-                &mut declarations.important
-            } else {
-                &mut declarations.normal
-            };
-
-            // Usually, the reference count will be 1 here. But transitions could make it greater
-            // than that.
-            let existing_declarations = Arc::make_mut(existing_declarations);
-            for declaration in &mut *existing_declarations {
-                if declaration.name() == property_decl.name() {
-                    *declaration = property_decl;
-                    return;
-                }
-            }
-            existing_declarations.push(property_decl);
             return;
         }
 
-        let (important, normal) = if style_priority == StylePriority::Important {
+        // explicitly not calling the push_new_attribute convenience method
-            (vec![property_decl], vec![])
+        // in order to avoid triggering mutation events
-        } else {
+        let window = window_from_node(self);
-            (vec![], vec![property_decl])
+        let attr = Attr::new(&window,
-        };
+                             atom!("style"),
+                             new_style,
+                             atom!("style"),
+                             ns!(),
+                             Some(atom!("style")),
+                             Some(self));
 
-        *inline_declarations = Some(PropertyDeclarationBlock {
+         assert!(attr.GetOwnerElement().r() == Some(self));
-            important: Arc::new(important),
+         self.attrs.borrow_mut().push(JS::from_ref(&attr));
-            normal: Arc::new(normal),
+    }
-        });
+
+    pub fn remove_inline_style_property(&self, property: &str) {
+        fn remove(element: &Element, property: &str) {
+            let mut inline_declarations = element.style_attribute.borrow_mut();
+            if let &mut Some(ref mut declarations) = &mut *inline_declarations {
+                let index = declarations.normal
+                                        .iter()
+                                        .position(|decl| decl.matches(property));
+                if let Some(index) = index {
+                    Arc::make_mut(&mut declarations.normal).remove(index);
+                    return;
+                }
+
+                let index = declarations.important
+                                        .iter()
+                                        .position(|decl| decl.matches(property));
+                if let Some(index) = index {
+                    Arc::make_mut(&mut declarations.important).remove(index);
+                    return;
+                }
+            }
+        }
+
+        remove(self, property);
+        self.sync_property_with_attrs_style();
+    }
+
+    pub fn update_inline_style(&self,
+                               declarations: Vec<PropertyDeclaration>,
+                               style_priority: StylePriority) {
+
+        fn update(element: &Element, mut declarations: Vec<PropertyDeclaration>, style_priority: StylePriority) {
+            let mut inline_declarations = element.style_attribute().borrow_mut();
+            if let &mut Some(ref mut existing_declarations) = &mut *inline_declarations {
+                let existing_declarations = if style_priority == StylePriority::Important {
+                    &mut existing_declarations.important
+                } else {
+                    &mut existing_declarations.normal
+                };
+
+                // Usually, the reference count will be 1 here. But transitions could make it greater
+                // than that.
+                let existing_declarations = Arc::make_mut(existing_declarations);
+
+                while let Some(mut incoming_declaration) = declarations.pop() {
+                    let mut replaced = false;
+                    for existing_declaration in &mut *existing_declarations {
+                        if existing_declaration.name() == incoming_declaration.name() {
+                            mem::swap(existing_declaration, &mut incoming_declaration);
+                            replaced = true;
+                            break;
+                        }
+                    }
+
+                    if !replaced {
+                        // inserting instead of pushing since the declarations are in reverse order
+                        existing_declarations.insert(0, incoming_declaration);
+                    }
+                }
+
+                return;
+            }
+
+            let (important, normal) = if style_priority == StylePriority::Important {
+                (declarations, vec![])
+            } else {
+                (vec![], declarations)
+            };
+
+            *inline_declarations = Some(PropertyDeclarationBlock {
+                important: Arc::new(important),
+                normal: Arc::new(normal),
+            });
+        }
+
+        update(self, declarations, style_priority);
+        self.sync_property_with_attrs_style();
     }
 
     pub fn set_inline_style_property_priority(&self,
                                               properties: &[&str],
                                               style_priority: StylePriority) {
-        let mut inline_declarations = self.style_attribute().borrow_mut();
+        {
-        if let &mut Some(ref mut declarations) = &mut *inline_declarations {
+            let mut inline_declarations = self.style_attribute().borrow_mut();
-            let (from, to) = if style_priority == StylePriority::Important {
+            if let &mut Some(ref mut declarations) = &mut *inline_declarations {
-                (&mut declarations.normal, &mut declarations.important)
+              let (from, to) = if style_priority == StylePriority::Important {
-            } else {
+                  (&mut declarations.normal, &mut declarations.important)
-                (&mut declarations.important, &mut declarations.normal)
+              } else {
-            };
+                  (&mut declarations.important, &mut declarations.normal)
+              };
 
-            // Usually, the reference counts of `from` and `to` will be 1 here. But transitions
+              // Usually, the reference counts of `from` and `to` will be 1 here. But transitions
-            // could make them greater than that.
+              // could make them greater than that.
-            let from = Arc::make_mut(from);
+              let from = Arc::make_mut(from);
-            let to = Arc::make_mut(to);
+              let to = Arc::make_mut(to);
-            let mut new_from = Vec::new();
+              let mut new_from = Vec::new();
-            for declaration in from.drain(..) {
+              for declaration in from.drain(..) {
-                let name = declaration.name();
+                  let name = declaration.name();
-                if properties.iter().any(|p| name == **p) {
+                  if properties.iter().any(|p| name == **p) {
-                    to.push(declaration)
+                      to.push(declaration)
-                } else {
+                  } else {
-                    new_from.push(declaration)
+                      new_from.push(declaration)
-                }
+                  }
+              }
+              mem::replace(from, new_from);
             }
-            mem::replace(from, new_from);
         }
+
+        self.sync_property_with_attrs_style();
     }
 
     pub fn get_inline_style_declaration(&self,

components/style/properties/properties.mako.rs

@@ -14,6 +14,7 @@ use std::ascii::AsciiExt;
 use std::boxed::Box as StdBox;
 use std::collections::HashSet;
 use std::fmt;
+use std::fmt::Write;
 use std::intrinsics;
 use std::mem;
 use std::sync::Arc;
@@ -253,8 +254,12 @@ mod property_bit_field {
     % endif
 % endfor
 
-/// Declarations are stored in reverse order.
+
+use std::iter::{Iterator, Chain, Zip, Rev, Repeat, repeat};
+use std::slice;
 /// Overridden declarations are skipped.
+
+// FIXME (https://github.com/servo/servo/issues/3426)
 #[derive(Debug, PartialEq, HeapSizeOf)]
 pub struct PropertyDeclarationBlock {
     #[ignore_heap_size_of = "#7038"]
@@ -263,6 +268,222 @@ pub struct PropertyDeclarationBlock {
     pub normal: Arc<Vec<PropertyDeclaration>>,
 }
 
+impl PropertyDeclarationBlock {
+    /// Provides an iterator of all declarations, with indication of !important value
+    pub fn declarations(&self) -> Chain<
+        Zip<Rev<slice::Iter<PropertyDeclaration>>, Repeat<bool>>,
+        Zip<Rev<slice::Iter<PropertyDeclaration>>, Repeat<bool>>
+    > {
+        // Declarations are stored in reverse order.
+        let normal = self.normal.iter().rev().zip(repeat(false));
+        let important = self.important.iter().rev().zip(repeat(true));
+        normal.chain(important)
+    }
+}
+
+impl ToCss for PropertyDeclarationBlock {
+    // https://drafts.csswg.org/cssom/#serialize-a-css-declaration-block
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+        let mut is_first_serialization = true; // trailing serializations should have a prepended space
+
+        // Step 1 -> dest = result list
+
+        // Step 2
+        let mut already_serialized = Vec::new();
+
+        // Step 3
+        for (declaration, important) in self.declarations() {
+            // Step 3.1
+            let property = declaration.name();
+
+            // Step 3.2
+            if already_serialized.contains(&property) {
+                continue;
+            }
+
+            // Step 3.3
+            let shorthands = declaration.shorthands();
+            if !shorthands.is_empty() {
+
+                // Step 3.3.1
+                let mut longhands = self.declarations()
+                    .filter(|d| !already_serialized.contains(&d.0.name()))
+                    .collect::<Vec<_>>();
+
+                // Step 3.3.2
+                for shorthand in shorthands {
+                    let properties = shorthand.longhands();
+
+                    // Substep 2 & 3
+                    let mut current_longhands = Vec::new();
+                    let mut important_count = 0;
+
+                    for &(longhand, longhand_important) in longhands.iter() {
+                        let longhand_name = longhand.name();
+                        if properties.iter().any(|p| &longhand_name == *p) {
+                            current_longhands.push(longhand);
+                            if longhand_important {
+                                important_count += 1;
+                            }
+                        }
+                    }
+
+                    // Substep 1
+                    /* Assuming that the PropertyDeclarationBlock contains no duplicate entries,
+                    if the current_longhands length is equal to the properties length, it means
+                    that the properties that map to shorthand are present in longhands */
+                    if current_longhands.is_empty() || current_longhands.len() != properties.len() {
+                        continue;
+                    }
+
+                    // Substep 4
+                    let is_important = important_count > 0;
+                    if is_important && important_count != current_longhands.len() {
+                        continue;
+                    }
+
+                    // TODO: serialize shorthand does not take is_important into account currently
+                    // Substep 5
+                    let was_serialized =
+                        try!(
+                            shorthand.serialize_shorthand_to_buffer(
+                                dest,
+                                current_longhands.iter().cloned(),
+                                &mut is_first_serialization
+                            )
+                        );
+                    // If serialization occured, Substep 7 & 8 will have been completed
+
+                    // Substep 6
+                    if !was_serialized {
+                        continue;
+                    }
+
+                    for current_longhand in current_longhands {
+                        // Substep 9
+                        already_serialized.push(current_longhand.name());
+                        let index_to_remove = longhands.iter().position(|l| l.0 == current_longhand);
+                        if let Some(index) = index_to_remove {
+                            // Substep 10
+                            longhands.remove(index);
+                        }
+                     }
+                 }
+            }
+
+            // Step 3.3.4
+            if already_serialized.contains(&property) {
+                continue;
+            }
+
+            use std::iter::Cloned;
+            use std::slice;
+
+            // Steps 3.3.5, 3.3.6 & 3.3.7
+            // Need to specify an iterator type here even though it’s unused to work around
+            // "error: unable to infer enough type information about `_`;
+            //  type annotations or generic parameter binding required [E0282]"
+            // Use the same type as earlier call to reuse generated code.
+            try!(append_serialization::<W, Cloned<slice::Iter< &PropertyDeclaration>>>(
+                dest,
+                &property.to_string(),
+                AppendableValue::Declaration(declaration),
+                important,
+                &mut is_first_serialization));
+
+            // Step 3.3.8
+            already_serialized.push(property);
+        }
+
+        // Step 4
+        Ok(())
+    }
+}
+
+enum AppendableValue<'a, I>
+where I: Iterator<Item=&'a PropertyDeclaration> {
+    Declaration(&'a PropertyDeclaration),
+    DeclarationsForShorthand(I),
+    Css(&'a str)
+}
+
+fn append_property_name<W>(dest: &mut W,
+                           property_name: &str,
+                           is_first_serialization: &mut bool)
+                           -> fmt::Result where W: fmt::Write {
+
+    // after first serialization(key: value;) add whitespace between the pairs
+    if !*is_first_serialization {
+        try!(write!(dest, " "));
+    }
+    else {
+        *is_first_serialization = false;
+    }
+
+    write!(dest, "{}", property_name)
+}
+
+fn append_declaration_value<'a, W, I>
+                           (dest: &mut W,
+                            appendable_value: AppendableValue<'a, I>,
+                            is_important: bool)
+                            -> fmt::Result
+                            where W: fmt::Write, I: Iterator<Item=&'a PropertyDeclaration> {
+  match appendable_value {
+      AppendableValue::Css(css) => {
+          try!(write!(dest, "{}", css))
+      },
+      AppendableValue::Declaration(decl) => {
+          try!(decl.to_css(dest));
+       },
+       AppendableValue::DeclarationsForShorthand(decls) => {
+           let mut decls = decls.peekable();
+           while let Some(decl) = decls.next() {
+               try!(decl.to_css(dest));
+
+               if decls.peek().is_some() {
+                   try!(write!(dest, " "));
+               }
+           }
+       }
+  }
+
+  if is_important {
+      try!(write!(dest, " !important"));
+  }
+
+  Ok(())
+}
+
+fn append_serialization<'a, W, I>(dest: &mut W,
+                                  property_name: &str,
+                                  appendable_value: AppendableValue<'a, I>,
+                                  is_important: bool,
+                                  is_first_serialization: &mut bool)
+                                  -> fmt::Result
+                                  where W: fmt::Write, I: Iterator<Item=&'a PropertyDeclaration> {
+
+    try!(append_property_name(dest, property_name, is_first_serialization));
+    try!(write!(dest, ":"));
+
+    // for normal parsed values, add a space between key: and value
+    match &appendable_value {
+        &AppendableValue::Css(_) => {
+            try!(write!(dest, " "))
+        },
+        &AppendableValue::Declaration(decl) => {
+            if !decl.value_is_unparsed() {
+                // for normal parsed values, add a space between key: and value
+                try!(write!(dest, " "));
+            }
+         },
+         &AppendableValue::DeclarationsForShorthand(_) => try!(write!(dest, " "))
+    }
+
+    try!(append_declaration_value(dest, appendable_value, is_important));
+    write!(dest, ";")
+}
+
 pub fn parse_style_attribute(input: &str, base_url: &Url, error_reporter: StdBox<ParseErrorReporter + Send>,
                              extra_data: ParserContextExtraData)
                              -> PropertyDeclarationBlock {
@@ -411,6 +632,14 @@ impl Shorthand {
         }
     }
 
+    pub fn name(&self) -> &'static str {
+        match *self {
+            % for property in data.shorthands:
+                Shorthand::${property.camel_case} => "${property.name}",
+            % endfor
+        }
+    }
+
     pub fn longhands(&self) -> &'static [&'static str] {
         % for property in data.shorthands:
             static ${property.ident.upper()}: &'static [&'static str] = &[
@@ -425,6 +654,71 @@ impl Shorthand {
             % endfor
         }
     }
+
+    /// Serializes possible shorthand value to String.
+    pub fn serialize_shorthand_value_to_string<'a, I>(self, declarations: I, is_important: bool) -> String
+    where I: Iterator<Item=&'a PropertyDeclaration> + Clone {
+        let appendable_value = self.get_shorthand_appendable_value(declarations).unwrap();
+        let mut result = String::new();
+        append_declaration_value(&mut result, appendable_value, is_important).unwrap();
+        result
+    }
+
+    /// Serializes possible shorthand name with value to input buffer given a list of longhand declarations.
+    /// On success, returns true if shorthand value is written and false if no shorthand value is present.
+    pub fn serialize_shorthand_to_buffer<'a, W, I>(self,
+                                                   dest: &mut W,
+                                                   declarations: I,
+                                                   is_first_serialization: &mut bool)
+                                                   -> Result<bool, fmt::Error>
+    where W: Write, I: Iterator<Item=&'a PropertyDeclaration> + Clone {
+        match self.get_shorthand_appendable_value(declarations) {
+            None => Ok(false),
+            Some(appendable_value) => {
+                let property_name = self.name();
+
+                append_serialization(
+                    dest,
+                    property_name,
+                    appendable_value,
+                    false,
+                    is_first_serialization
+                ).and_then(|_| Ok(true))
+            }
+        }
+    }
+
+    fn get_shorthand_appendable_value<'a, I>(self, declarations: I) -> Option<AppendableValue<'a, I>>
+        where I: Iterator<Item=&'a PropertyDeclaration> + Clone {
+
+            // Only cloning iterators (a few pointers each) not declarations.
+            let mut declarations2 = declarations.clone();
+            let mut declarations3 = declarations.clone();
+
+            let first_declaration = match declarations2.next() {
+                Some(declaration) => declaration,
+                None => return None
+            };
+
+            // https://drafts.csswg.org/css-variables/#variables-in-shorthands
+            if let Some(css) = first_declaration.with_variables_from_shorthand(self) {
+                if declarations2.all(|d| d.with_variables_from_shorthand(self) == Some(css)) {
+                   return Some(AppendableValue::Css(css));
+               }
+               else {
+                   return None;
+               }
+            }
+
+            if !declarations3.any(|d| d.with_variables()) {
+                return Some(AppendableValue::DeclarationsForShorthand(declarations));
+                // 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
+            }
+
+            None
+    }
 }
 
 #[derive(Clone, PartialEq, Eq, Debug, HeapSizeOf)]
@@ -506,6 +800,22 @@ impl fmt::Display for PropertyDeclarationName {
         }
     }
 }
+impl ToCss for PropertyDeclaration {
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+        match *self {
+            % for property in data.longhands:
+                % if not property.derived_from:
+                    PropertyDeclaration::${property.camel_case}(ref value) =>
+                        value.to_css(dest),
+                % endif
+            % endfor
+            PropertyDeclaration::Custom(_, ref value) => value.to_css(dest),
+            % if any(property.derived_from for property in data.longhands):
+                _ => Err(fmt::Error),
+            % endif
+        }
+    }
+}
 
 impl PropertyDeclaration {
     pub fn name(&self) -> PropertyDeclarationName {
@@ -525,17 +835,12 @@ impl PropertyDeclaration {
     }
 
     pub fn value(&self) -> String {
-        match *self {
+        let mut value = String::new();
-            % for property in data.longhands:
+        if let Err(_) = self.to_css(&mut value) {
-                PropertyDeclaration::${property.camel_case}
+            panic!("unsupported property declaration: {}", self.name());
-                % if not property.derived_from:
-                    (ref value) => value.to_css_string(),
-                % else:
-                    (_) => panic!("unsupported property declaration: ${property.name}"),
-                % endif
-            % endfor
-            PropertyDeclaration::Custom(_, ref value) => value.to_css_string(),
         }
+
+        value
     }
 
     /// If this is a pending-substitution value from the given shorthand, return that value
@@ -573,6 +878,20 @@ impl PropertyDeclaration {
         }
     }
 
+    /// Return whether the value is stored as it was in the CSS source, preserving whitespace
+    /// (as opposed to being parsed into a more abstract data structure).
+    /// This is the case of custom properties and values that contain unsubstituted variables.
+    pub fn value_is_unparsed(&self) -> bool {
+      match *self {
+          % for property in data.longhands:
+              PropertyDeclaration::${property.camel_case}(ref value) => {
+                  matches!(*value, DeclaredValue::WithVariables { .. })
+              },
+          % endfor
+          PropertyDeclaration::Custom(..) => true
+      }
+    }
+
     pub fn matches(&self, name: &str) -> bool {
         match *self {
             % for property in data.longhands:
@@ -680,6 +999,38 @@ impl PropertyDeclaration {
             _ => PropertyDeclarationParseResult::UnknownProperty
         }
     }
+
+    pub fn shorthands(&self) -> &'static [Shorthand] {
+        // first generate longhand to shorthands lookup map
+        <%
+            longhand_to_shorthand_map = {}
+            for shorthand in data.shorthands:
+                for sub_property in shorthand.sub_properties:
+                    if sub_property.ident not in longhand_to_shorthand_map:
+                        longhand_to_shorthand_map[sub_property.ident] = []
+
+                    longhand_to_shorthand_map[sub_property.ident].append(shorthand.camel_case)
+
+            for shorthand_list in longhand_to_shorthand_map.itervalues():
+                shorthand_list.sort()
+        %>
+
+        // based on lookup results for each longhand, create result arrays
+        % for property in data.longhands:
+            static ${property.ident.upper()}: &'static [Shorthand] = &[
+                % for shorthand in longhand_to_shorthand_map.get(property.ident, []):
+                    Shorthand::${shorthand},
+                % endfor
+            ];
+        % endfor
+
+        match *self {
+            % for property in data.longhands:
+                PropertyDeclaration::${property.camel_case}(_) => ${property.ident.upper()},
+            % endfor
+            PropertyDeclaration::Custom(_, _) => &[]
+        }
+    }
 }
 
 pub mod style_struct_traits {

tests/unit/style/properties.rs

@@ -2,11 +2,16 @@
  * 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::ToCss;
 use rustc_serialize::json::Json;
 use std::env;
 use std::fs::{File, remove_file};
 use std::path::Path;
 use std::process::Command;
+use std::sync::Arc;
+use style::computed_values::display::T::inline_block;
+use style::properties::{PropertyDeclaration, PropertyDeclarationBlock, DeclaredValue};
+use style::values::specified::{Length, LengthOrPercentageOrAuto, LengthOrPercentage};
 
 #[test]
 fn properties_list_json() {
@@ -51,3 +56,38 @@ fn find_python() -> String {
         "python"
     }.to_owned()
 }
+
+#[test]
+fn property_declaration_block_should_serialize_correctly() {
+    let mut normal = Vec::new();
+    let mut important = Vec::new();
+
+    let length = LengthOrPercentageOrAuto::Length(Length::from_px(70f32));
+    let value = DeclaredValue::Value(length);
+    normal.push(PropertyDeclaration::Width(value));
+
+    let min_height = LengthOrPercentage::Length(Length::from_px(20f32));
+    let value = DeclaredValue::Value(min_height);
+    normal.push(PropertyDeclaration::MinHeight(value));
+
+    let value = DeclaredValue::Value(inline_block);
+    normal.push(PropertyDeclaration::Display(value));
+
+    let height = LengthOrPercentageOrAuto::Length(Length::from_px(20f32));
+    let value = DeclaredValue::Value(height);
+    important.push(PropertyDeclaration::Height(value));
+
+    normal.reverse();
+    important.reverse();
+    let block = PropertyDeclarationBlock {
+        normal: Arc::new(normal),
+        important: Arc::new(important)
+    };
+
+    let css_string = block.to_css_string();
+
+    assert_eq!(
+        css_string,
+        "width: 70px; min-height: 20px; display: inline-block; height: 20px !important;"
+    );
+}