Property declaration block serialization should check for importance

components/style/properties/declaration_block.rs

@@ -81,6 +81,7 @@ impl PropertyDeclarationBlock {
         if let Some(shorthand) = Shorthand::from_name(&property) {
             // Step 2.1
             let mut list = Vec::new();
+            let mut important_count = 0;
 
             // Step 2.2
             for longhand in shorthand.longhands() {
@@ -89,13 +90,26 @@ impl PropertyDeclarationBlock {
 
                 // Step 2.2.2 & 2.2.3
                 match declaration {
-                    Some(&(ref declaration, _importance)) => list.push(declaration),
+                    Some(&(ref declaration, importance)) => {
+                        list.push(declaration);
+                        if importance.important() {
+                            important_count += 1;
+                        }
+                    },
                     None => return Ok(()),
                 }
             }
 
+            // Step 3.3.2.4
+            // If there is one or more longhand with important, and one or more
+            // without important, we don't serialize it as a shorthand.
+            if important_count > 0 && important_count != list.len() {
+                return Ok(());
+            }
+
             // Step 2.3
-            // TODO: importance is hardcoded because method does not implement it yet
+            // We don't print !important when serializing individual properties,
+            // so we treat this as a normal-importance property
             let importance = Importance::Normal;
             let appendable_value = shorthand.get_shorthand_appendable_value(list).unwrap();
             return append_declaration_value(dest, appendable_value, importance)
@@ -286,15 +300,20 @@ impl ToCss for PropertyDeclarationBlock {
                     if is_important && important_count != current_longhands.len() {
                         continue;
                     }
+                    let importance = if is_important {
+                        Importance::Important
+                    } else {
+                        Importance::Normal
+                    };
 
-                    // 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
+                                &mut is_first_serialization,
+                                importance
                             )
                         );
                     // If serialization occured, Substep 7 & 8 will have been completed

components/style/properties/properties.mako.rs

@@ -435,7 +435,8 @@ impl Shorthand {
     pub fn serialize_shorthand_to_buffer<'a, W, I>(self,
                                                    dest: &mut W,
                                                    declarations: I,
-                                                   is_first_serialization: &mut bool)
+                                                   is_first_serialization: &mut bool,
+                                                   importance: Importance)
                                                    -> Result<bool, fmt::Error>
     where W: Write, I: IntoIterator<Item=&'a PropertyDeclaration>, I::IntoIter: Clone {
         match self.get_shorthand_appendable_value(declarations) {
@@ -447,7 +448,7 @@ impl Shorthand {
                     dest,
                     property_name,
                     appendable_value,
-                    Importance::Normal,
+                    importance,
                     is_first_serialization
                 ).and_then(|_| Ok(true))
             }

tests/wpt/metadata/MANIFEST.json

@@ -39677,6 +39677,12 @@
     "deleted_reftests": {},
     "items": {
       "testharness": {
+        "cssom/shorthand-serialization.html": [
+          {
+            "path": "cssom/shorthand-serialization.html",
+            "url": "/cssom/shorthand-serialization.html"
+          }
+        ],
         "html/semantics/forms/form-submission-0/submit-entity-body.html": [
           {
             "path": "html/semantics/forms/form-submission-0/submit-entity-body.html",

tests/wpt/web-platform-tests/cssom/shorthand-serialization.html

@@ -0,0 +1,44 @@
+<!doctype html>
+<html>
+<head>
+    <meta charset="utf-8">
+    <title>Shorthand serialization should be done correctly.</title>
+    <script src="/resources/testharness.js"></script>
+    <script src="/resources/testharnessreport.js"></script>
+</head>
+<body>
+    <div id="foo1" style="background: red;">foo</div>
+    <div id="foo2" style="background-color: blue; background: red !important; background-color: green;">foo</div>
+    <div id="foo3" style="background-color: blue; background: red; background-color: green !important;">foo</div>
+
+    <div id="foo4" style="margin-right: 10px; margin-left: 10px; margin-top: 10px; margin-bottom: 10px;">foo</div>
+    <div id="foo5" style="margin-right: 10px; margin-left: 10px; margin-top: 10px; margin-bottom: 10px!important;">foo</div>
+    <div id="foo6" style="margin-right: 10px !important; margin-left: 10px !important; margin-top: 10px !important; margin-bottom: 10px!important;">foo</div>
+
+    <script>
+        test(function() {
+            var elem1 = document.getElementById('foo1');
+            var elem2 = document.getElementById('foo2');
+            var elem3 = document.getElementById('foo3');
+
+            assert_false(elem1.style.cssText.endsWith('!important;'));
+            assert_true(elem2.style.cssText.endsWith('!important;'));
+            assert_false(elem3.style.background.endsWith('!important'));
+
+        }, "Shorthand serialization with shorthand and longhands mixed.");
+
+        test(function() {
+            var elem4 = document.getElementById('foo4');
+            var elem5 = document.getElementById('foo5');
+            var elem6 = document.getElementById('foo6');
+
+            assert_equals(elem4.style.cssText, 'margin: 10px;');
+            assert_equals(elem4.style.margin, '10px');
+            assert_equals(elem5.style.cssText, 'margin-right: 10px; margin-left: 10px; margin-top: 10px; margin-bottom: 10px !important;');
+            assert_equals(elem5.style.margin, '');
+            assert_equals(elem6.style.cssText, 'margin: 10px !important;');
+            assert_equals(elem6.style.margin, '10px');
+        }, "Shorthand serialization with just longhands.");
+    </script>
+</body>
+</html>