style: Multiple calc serialization fixes.

components/style/Cargo.toml

@@ -29,7 +29,7 @@ servo = ["serde", "heapsize", "heapsize_derive",
 gecko_debug = ["nsstring_vendor/gecko_debug"]
 
 [dependencies]
-app_units = "0.5.2"
+app_units = "0.5.3"
 arrayvec = "0.3.20"
 arraydeque = "0.2.3"
 atomic_refcell = "0.1"

components/style/values/computed/length.rs

@@ -194,11 +194,21 @@ impl From<LengthOrPercentageOrNone> for Option<CalcLengthOrPercentage> {
 
 impl ToCss for CalcLengthOrPercentage {
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-        match (self.length, self.percentage) {
-            (l, Some(p)) if l == Au(0) => p.to_css(dest),
-            (l, Some(p)) => write!(dest, "calc({}px + {}%)", Au::to_px(l), p.0 * 100.),
-            (l, None) => write!(dest, "{}px", Au::to_px(l)),
-        }
+        use num_traits::Zero;
+
+        let (length, percentage) = match (self.length, self.percentage) {
+            (l, None) => return l.to_css(dest),
+            (l, Some(p)) if l == Au(0) => return p.to_css(dest),
+            (l, Some(p)) => (l, p),
+        };
+
+        dest.write_str("calc(")?;
+        percentage.to_css(dest)?;
+
+        dest.write_str(if length < Zero::zero() { " - " } else { " + " })?;
+        length.abs().to_css(dest)?;
+
+        dest.write_str(")")
     }
 }
 

components/style/values/computed/percentage.rs

@@ -11,7 +11,7 @@ use values::CSSFloat;
 use values::animated::ToAnimatedZero;
 
 /// A computed percentage.
-#[derive(Clone, ComputeSquaredDistance, Copy, Debug, Default, HasViewportPercentage, PartialEq)]
+#[derive(Clone, ComputeSquaredDistance, Copy, Debug, Default, HasViewportPercentage, PartialEq, PartialOrd)]
 #[cfg_attr(feature = "servo", derive(Deserialize, HeapSizeOf, Serialize))]
 pub struct Percentage(pub CSSFloat);
 
@@ -27,6 +27,12 @@ impl Percentage {
     pub fn hundred() -> Self {
         Percentage(1.)
     }
+
+    /// Returns the absolute value for this percentage.
+    #[inline]
+    pub fn abs(&self) -> Self {
+        Percentage(self.0.abs())
+    }
 }
 
 /// https://drafts.csswg.org/css-transitions/#animtype-percentage

components/style/values/specified/calc.rs

@@ -90,25 +90,35 @@ impl HasViewportPercentage for CalcLengthOrPercentage {
 }
 
 impl ToCss for CalcLengthOrPercentage {
+    /// https://drafts.csswg.org/css-values/#calc-serialize
+    ///
+    /// FIXME(emilio): Should this simplify away zeros?
     #[allow(unused_assignments)]
     fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
+        use num_traits::Zero;
+
         let mut first_value = true;
         macro_rules! first_value_check {
-            () => {
+            ($val:expr) => {
                 if !first_value {
-                    dest.write_str(" + ")?;
-                } else {
-                    first_value = false;
+                    dest.write_str(if $val < Zero::zero() {
+                        " - "
+                    } else {
+                        " + "
+                    })?;
+                } else if $val < Zero::zero() {
+                    dest.write_str("-")?;
                 }
+                first_value = false;
             };
         }
 
         macro_rules! serialize {
             ( $( $val:ident ),* ) => {
                 $(
                     if let Some(val) = self.$val {
-                        first_value_check!();
-                        val.to_css(dest)?;
+                        first_value_check!(val);
+                        val.abs().to_css(dest)?;
                         dest.write_str(stringify!($val))?;
                     }
                 )*
@@ -117,24 +127,30 @@ impl ToCss for CalcLengthOrPercentage {
 
         dest.write_str("calc(")?;
 
-        serialize!(ch, em, ex, rem, vh, vmax, vmin, vw);
+        // NOTE(emilio): Percentages first because of web-compat problems, see:
+        // https://github.com/w3c/csswg-drafts/issues/1731
+        if let Some(val) = self.percentage {
+            first_value_check!(val.0);
+            val.abs().to_css(dest)?;
+        }
+
+        // NOTE(emilio): The order here it's very intentional, and alphabetic
+        // per the spec linked above.
+        serialize!(ch, em, ex);
 
         #[cfg(feature = "gecko")]
         {
             serialize!(mozmm);
         }
 
         if let Some(val) = self.absolute {
-            first_value_check!();
-            val.to_css(dest)?;
+            first_value_check!(val);
+            val.abs().to_css(dest)?;
         }
 
-        if let Some(val) = self.percentage {
-            first_value_check!();
-            val.to_css(dest)?;
-        }
+        serialize!(rem, vh, vmax, vmin, vw);
 
-        write!(dest, ")")
+        dest.write_str(")")
     }
 }
 

tests/wpt/mozilla/meta/MANIFEST.json

@@ -13512,6 +13512,12 @@
      {}
     ]
    ],
+   "css/calc-serialization.html": [
+    [
+     "/_mozilla/css/calc-serialization.html",
+     {}
+    ]
+   ],
    "css/empty-keyframes.html": [
     [
      "/_mozilla/css/empty-keyframes.html",
@@ -22847,6 +22853,10 @@
    "b9e77c824d0c96e51d51fb8f1e923d3ab67be027",
    "testharness"
   ],
+  "css/calc-serialization.html": [
+   "628597eb36bf21a1ec982c7f6935ee7949c62044",
+   "testharness"
+  ],
   "css/canvas_as_block_element_a.html": [
    "668d93da2dab9722998cc7c5785c20e2ab9a1ced",
    "reftest"
@@ -26508,7 +26518,7 @@
    "testharness"
   ],
   "mozilla/calc.html": [
-   "5d59d5b367a023e778ce6968acdeca6b8b7c3197",
+   "5b3ea33205a9f2404bd6976a2c7f6fc451be8e96",
    "testharness"
   ],
   "mozilla/canvas.initial.reset.2dstate.html": [

tests/wpt/mozilla/tests/css/calc-serialization.html

@@ -0,0 +1,34 @@
+<!doctype html>
+<meta charset="utf-8">
+<title>CSS Values and Units: calc() serialization.</title>
+<link rel="author" title="Emilio Cobos Álvarez" href="mailto:emilio@mozilla.com">
+<link rel="help" href="https://drafts.csswg.org/css-values/#calc-serialize">
+<script src="/resources/testharness.js"></script>
+<script src="/resources/testharnessreport.js"></script>
+<!-- FIXME(emilio): Upstream when spec stops being under discussion -->
+<div id="content"></div>
+<script>
+
+// NOTE(emilio): This intentionally doesn't test for order of percentages
+// because of:
+//
+// https://github.com/w3c/csswg-drafts/issues/1731
+
+test(function() {
+  // specified -> expected
+  var values = {
+    "calc(10px + 1vmin)": "calc(10px + 1vmin)",
+    "calc(10px + 1em)": "calc(1em + 10px)",
+    "calc(1vmin - 10px)": "calc(-10px + 1vmin)",
+    "calc(-10px + 1em)": "calc(1em - 10px)",
+    "calc(-10px)": "calc(-10px)",
+  };
+
+  var content = document.getElementById("content");
+
+  for (var prop in values) {
+    content.style.width = prop;
+    assert_equals(content.style.width, values[prop], "Serialization of " + prop);
+  }
+}, "calc() serialization")
+</script>

tests/wpt/mozilla/tests/mozilla/calc.html

@@ -46,13 +46,13 @@
 
     // Alphabetical order
     ['calc(0ch + 0px + 0pt + 0pc + 0in + 0cm + 0mm + 0rem + 0em + 0ex + 0% + 0vw + 0vh + 0vmin + 0vmax)',
-     'calc(0ch + 0em + 0ex + 0rem + 0vh + 0vmax + 0vmin + 0vw + 0px + 0%)',
+     'calc(0% + 0ch + 0em + 0ex + 0px + 0rem + 0vh + 0vmax + 0vmin + 0vw)',
      '0px'],
 
     // Simplification
     ['calc((2 - 1) * 10px)', 'calc(10px)', '10px'],
     ['calc(((3 - 1) * (8 + 4)) * 10px)', 'calc(240px)', '240px'],
-    ['calc(5 * (20px / 2 + 7 * (3em + 12px/4 + (8 - 2) * 2rem)))', 'calc(105em + 420rem + 155px)', '8555px'],
+    ['calc(5 * (20px / 2 + 7 * (3em + 12px/4 + (8 - 2) * 2rem)))', 'calc(105em + 155px + 420rem)', '8555px'],
 
 ];
 
@@ -107,7 +107,7 @@
 lengthOrPercentageProperties.forEach(function(prop) {
     test(function() {
         div.style.setProperty(prop, 'calc(1px + 0%)');
-        assert_equals(div.style.getPropertyValue(prop), 'calc(1px + 0%)');
+        assert_equals(div.style.getPropertyValue(prop), 'calc(0% + 1px)');
     }, 'calc for ' + prop);
 });
 
@@ -142,14 +142,14 @@
 var otherProperties = [
   ['border-width', 'calc(1px)', 'calc(1px)'],
   ['border-spacing', 'calc(1px)', 'calc(1px)'],
-  ['transform-origin', 'calc(1px + 0%)', 'calc(1px + 0%) center 0px'],
-  ['perspective-origin', 'calc(1px + 0%)', 'calc(1px + 0%) center'],
-  ['background-size', 'calc(1px + 0%)', 'calc(1px + 0%) auto'],
-  ['background-position', 'calc(1px + 0%) calc(2px + 0%)', 'calc(1px + 0%) calc(2px + 0%)'],
-  ['border-top-left-radius', 'calc(1px + 0%)', 'calc(1px + 0%) calc(1px + 0%)'],
-  ['border-bottom-left-radius', 'calc(1px + 0%)', 'calc(1px + 0%) calc(1px + 0%)'],
-  ['border-top-right-radius', 'calc(1px + 0%)', 'calc(1px + 0%) calc(1px + 0%)'],
-  ['border-bottom-right-radius', 'calc(1px + 0%)', 'calc(1px + 0%) calc(1px + 0%)'],
+  ['transform-origin', 'calc(1px + 0%)', 'calc(0% + 1px) center 0px'],
+  ['perspective-origin', 'calc(1px + 0%)', 'calc(0% + 1px) center'],
+  ['background-size', 'calc(1px + 0%)', 'calc(0% + 1px) auto'],
+  ['background-position', 'calc(1px + 0%) calc(2px + 0%)', 'calc(0% + 1px) calc(0% + 2px)'],
+  ['border-top-left-radius', 'calc(1px + 0%)', 'calc(0% + 1px) calc(0% + 1px)'],
+  ['border-bottom-left-radius', 'calc(1px + 0%)', 'calc(0% + 1px) calc(0% + 1px)'],
+  ['border-top-right-radius', 'calc(1px + 0%)', 'calc(0% + 1px) calc(0% + 1px)'],
+  ['border-bottom-right-radius', 'calc(1px + 0%)', 'calc(0% + 1px) calc(0% + 1px)'],
   ['counter-increment', 'foo calc(1 + 1)', 'foo calc(2)'],
 ];