style: Percentage values of translate are serialized as percent for computed values.

Basically, we rewrite the type of generics::transform::Translate and its
ToCss to match the spec. Besides, we always serialize Translate by servo,
so we could drop a lot of duplicated code.

Differential Revision: https://phabricator.services.mozilla.com/D11206

7 files changed, 127 insertions, 34 deletions

diff --git a/components/style/properties/helpers/animated_properties.mako.rs b/components/style/properties/helpers/animated_properties.mako.rs
index 284e35877df..5ff4a10c3c0 100644
--- a/components/style/properties/helpers/animated_properties.mako.rs
+++ b/components/style/properties/helpers/animated_properties.mako.rs
@@ -2102,12 +2102,12 @@ impl Matrix3D {
 
 /// <https://drafts.csswg.org/css-transforms-2/#propdef-rotate>
 impl ComputedRotate {
-    fn resolve(rotate: &ComputedRotate) -> (Number, Number, Number, Angle) {
+    fn resolve(&self) -> (Number, Number, Number, Angle) {
         // According to the spec:
         // https://drafts.csswg.org/css-transforms-2/#individual-transforms
         //
         // If the axis is unspecified, it defaults to "0 0 1"
-        match *rotate {
+        match *self {
             Rotate::None => (0., 0., 1., Angle::zero()),
             Rotate::Rotate3D(rx, ry, rz, angle) => (rx, ry, rz, angle),
             Rotate::Rotate(angle) => (0., 0., 1., angle),
@@ -2122,8 +2122,7 @@ impl Animate for ComputedRotate {
         other: &Self,
         procedure: Procedure,
     ) -> Result<Self, ()> {
-        let from = ComputedRotate::resolve(self);
-        let to = ComputedRotate::resolve(other);
+        let (from, to) = (self.resolve(), other.resolve());
 
         let (mut fx, mut fy, mut fz, fa) =
             transform::get_normalized_vector_and_angle(from.0, from.1, from.2, from.3);
@@ -2163,24 +2162,17 @@ impl Animate for ComputedRotate {
 
 /// <https://drafts.csswg.org/css-transforms-2/#propdef-translate>
 impl ComputedTranslate {
-    fn resolve(
-        translate: &ComputedTranslate,
-    ) -> (LengthOrPercentage, LengthOrPercentage, Length) {
+    fn resolve(&self) -> (LengthOrPercentage, LengthOrPercentage, Length) {
         // According to the spec:
         // https://drafts.csswg.org/css-transforms-2/#individual-transforms
         //
         // Unspecified translations default to 0px
-        match *translate {
+        match *self {
             Translate::None => {
-                (
-                    LengthOrPercentage::Length(Length::zero()),
-                    LengthOrPercentage::Length(Length::zero()),
-                    Length::zero(),
-                )
+                (LengthOrPercentage::zero(), LengthOrPercentage::zero(), Length::zero())
             },
             Translate::Translate3D(tx, ty, tz) => (tx, ty, tz),
             Translate::Translate(tx, ty) => (tx, ty, Length::zero()),
-            Translate::TranslateX(tx) => (tx, LengthOrPercentage::Length(Length::zero()), Length::zero()),
         }
     }
 }
@@ -2192,23 +2184,31 @@ impl Animate for ComputedTranslate {
         other: &Self,
         procedure: Procedure,
     ) -> Result<Self, ()> {
-        let from = ComputedTranslate::resolve(self);
-        let to = ComputedTranslate::resolve(other);
-
-        Ok(Translate::Translate3D(from.0.animate(&to.0, procedure)?,
-                                  from.1.animate(&to.1, procedure)?,
-                                  from.2.animate(&to.2, procedure)?))
+        match (self, other) {
+            (&Translate::None, &Translate::None) => Ok(Translate::None),
+            (&Translate::Translate3D(_, ..), _) | (_, &Translate::Translate3D(_, ..)) => {
+                let (from, to) = (self.resolve(), other.resolve());
+                Ok(Translate::Translate3D(from.0.animate(&to.0, procedure)?,
+                                          from.1.animate(&to.1, procedure)?,
+                                          from.2.animate(&to.2, procedure)?))
+            },
+            (&Translate::Translate(_, ..), _) | (_, &Translate::Translate(_, ..)) => {
+                let (from, to) = (self.resolve(), other.resolve());
+                Ok(Translate::Translate(from.0.animate(&to.0, procedure)?,
+                                        from.1.animate(&to.1, procedure)?))
+            },
+        }
     }
 }
 
 /// <https://drafts.csswg.org/css-transforms-2/#propdef-scale>
 impl ComputedScale {
-    fn resolve(scale: &ComputedScale) -> (Number, Number, Number) {
+    fn resolve(&self) -> (Number, Number, Number) {
         // According to the spec:
         // https://drafts.csswg.org/css-transforms-2/#individual-transforms
         //
         // Unspecified scales default to 1
-        match *scale {
+        match *self {
             Scale::None => (1.0, 1.0, 1.0),
             Scale::Scale3D(sx, sy, sz) => (sx, sy, sz),
             Scale::Scale(sx, sy) => (sx, sy, 1.),
@@ -2226,8 +2226,7 @@ impl Animate for ComputedScale {
         match (self, other) {
             (&Scale::None, &Scale::None) => Ok(Scale::None),
             (&Scale::Scale3D(_, ..), _) | (_, &Scale::Scale3D(_, ..)) => {
-                let from = ComputedScale::resolve(self);
-                let to = ComputedScale::resolve(other);
+                let (from, to) = (self.resolve(), other.resolve());
                 // FIXME(emilio, bug 1464791): why does this do something different than
                 // Scale3D / TransformOperation::Scale3D?
                 if procedure == Procedure::Add {
@@ -2241,8 +2240,7 @@ impl Animate for ComputedScale {
                 ))
             },
             (&Scale::Scale(_, ..), _) | (_, &Scale::Scale(_, ..)) => {
-                let from = ComputedScale::resolve(self);
-                let to = ComputedScale::resolve(other);
+                let (from, to) = (self.resolve(), other.resolve());
                 // FIXME(emilio, bug 1464791): why does this do something different than
                 // Scale / TransformOperation::Scale?
                 if procedure == Procedure::Add {
diff --git a/components/style/properties/longhands/box.mako.rs b/components/style/properties/longhands/box.mako.rs
index c06ed16a22e..685e97aa3b0 100644
--- a/components/style/properties/longhands/box.mako.rs
+++ b/components/style/properties/longhands/box.mako.rs
@@ -370,7 +370,7 @@ ${helpers.predefined_type(
     "generics::transform::Translate::None",
     animation_value_type="ComputedValue",
     boxed=True,
-    flags="CREATES_STACKING_CONTEXT FIXPOS_CB GETCS_NEEDS_LAYOUT_FLUSH",
+    flags="CREATES_STACKING_CONTEXT FIXPOS_CB",
     gecko_pref="layout.css.individual-transform.enabled",
     spec="https://drafts.csswg.org/css-transforms-2/#individual-transforms",
     servo_restyle_damage="reflow_out_of_flow",
diff --git a/components/style/values/computed/length.rs b/components/style/values/computed/length.rs
index 4c7fe16dcfc..6e5488fd42f 100644
--- a/components/style/values/computed/length.rs
+++ b/components/style/values/computed/length.rs
@@ -11,6 +11,7 @@ use crate::values::distance::{ComputeSquaredDistance, SquaredDistance};
 use crate::values::generics::length::{
     MaxLength as GenericMaxLength, MozLength as GenericMozLength,
 };
+use crate::values::generics::transform::IsZeroLength;
 use crate::values::generics::NonNegative;
 use crate::values::specified::length::ViewportPercentageLength;
 use crate::values::specified::length::{AbsoluteLength, FontBaseSize, FontRelativeLength};
@@ -496,6 +497,17 @@ impl ToComputedValue for specified::LengthOrPercentage {
     }
 }
 
+impl IsZeroLength for LengthOrPercentage {
+    #[inline]
+    fn is_zero_length(&self) -> bool {
+        match *self {
+            LengthOrPercentage::Length(l) => l.0 == 0.0,
+            LengthOrPercentage::Percentage(p) => p.0 == 0.0,
+            LengthOrPercentage::Calc(c) => c.unclamped_length().0 == 0.0 && c.percentage() == 0.0,
+        }
+    }
+}
+
 #[allow(missing_docs)]
 #[animate(fallback = "Self::animate_fallback")]
 #[css(derive_debug)]
diff --git a/components/style/values/computed/transform.rs b/components/style/values/computed/transform.rs
index 079c1e2535b..b4493265aef 100644
--- a/components/style/values/computed/transform.rs
+++ b/components/style/values/computed/transform.rs
@@ -334,7 +334,6 @@ impl Translate {
     pub fn to_transform_operation(&self) -> Option<TransformOperation> {
         match *self {
             generic::Translate::None => None,
-            generic::Translate::TranslateX(tx) => Some(generic::TransformOperation::TranslateX(tx)),
             generic::Translate::Translate(tx, ty) => {
                 Some(generic::TransformOperation::Translate(tx, Some(ty)))
             },
@@ -347,7 +346,6 @@ impl Translate {
     /// Convert Translate to TransformOperation.
     pub fn from_transform_operation(operation: &TransformOperation) -> Translate {
         match *operation {
-            generic::TransformOperation::TranslateX(tx) => generic::Translate::TranslateX(tx),
             generic::TransformOperation::Translate(tx, Some(ty)) => {
                 generic::Translate::Translate(tx, ty)
             },
diff --git a/components/style/values/generics/transform.rs b/components/style/values/generics/transform.rs
index c48c37e552a..12cc97fb10f 100644
--- a/components/style/values/generics/transform.rs
+++ b/components/style/values/generics/transform.rs
@@ -610,7 +610,6 @@ impl<Number: ToCss + PartialEq> ToCss for Scale<Number> {
     SpecifiedValueInfo,
     ToAnimatedZero,
     ToComputedValue,
-    ToCss,
 )]
 /// A value of the `Translate` property
 ///
@@ -618,14 +617,56 @@ impl<Number: ToCss + PartialEq> ToCss for Scale<Number> {
 pub enum Translate<LengthOrPercentage, Length> {
     /// 'none'
     None,
-    /// '<length-percentage>'
-    TranslateX(LengthOrPercentage),
-    /// '<length-percentage> <length-percentage>'
+    /// '<length-percentage>' or '<length-percentage> <length-percentage>'
     Translate(LengthOrPercentage, LengthOrPercentage),
     /// '<length-percentage> <length-percentage> <length>'
     Translate3D(LengthOrPercentage, LengthOrPercentage, Length),
 }
 
+/// A trait to check if this is a zero length.
+/// An alternative way is use num_traits::Zero. However, in order to implement num_traits::Zero,
+/// we also have to implement Add, which may be complicated for LengthOrPercentage::Calc.
+/// We could do this if other types also need it. If so, we could drop this trait.
+pub trait IsZeroLength {
+    /// Returns true if this is a zero length.
+    fn is_zero_length(&self) -> bool;
+}
+
+impl<LoP: ToCss + IsZeroLength, L: ToCss> ToCss for Translate<LoP, L> {
+    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
+    where
+        W: fmt::Write,
+    {
+        // The spec says:
+        // 1. If a 2d translation is specified, the property must serialize with only one or two
+        //    values (per usual, if the second value is 0px, the default, it must be omitted when
+        //    serializing).
+        // 2. If a 3d translation is specified, all three values must be serialized.
+        // https://drafts.csswg.org/css-transforms-2/#individual-transform-serialization
+        //
+        // We don't omit the 3rd component even if it is 0px for now, and the related
+        // spec issue is https://github.com/w3c/csswg-drafts/issues/3305
+        match *self {
+            Translate::None => dest.write_str("none"),
+            Translate::Translate(ref x, ref y) => {
+                x.to_css(dest)?;
+                if !y.is_zero_length() {
+                    dest.write_char(' ')?;
+                    y.to_css(dest)?;
+                }
+                Ok(())
+            },
+            Translate::Translate3D(ref x, ref y, ref z) => {
+                x.to_css(dest)?;
+                dest.write_char(' ')?;
+                y.to_css(dest)?;
+                dest.write_char(' ')?;
+                z.to_css(dest)
+            },
+        }
+    }
+}
+
 #[allow(missing_docs)]
 #[derive(
     Clone, Copy, Debug, MallocSizeOf, Parse, PartialEq, SpecifiedValueInfo, ToComputedValue, ToCss,
diff --git a/components/style/values/specified/length.rs b/components/style/values/specified/length.rs
index 4dd4ab42935..429d06a1e54 100644
--- a/components/style/values/specified/length.rs
+++ b/components/style/values/specified/length.rs
@@ -14,6 +14,7 @@ use crate::values::computed::{self, CSSPixelLength, Context, ExtremumLength};
 use crate::values::generics::length::{
     MaxLength as GenericMaxLength, MozLength as GenericMozLength,
 };
+use values::generics::transform::IsZeroLength;
 use crate::values::generics::NonNegative;
 use crate::values::specified::calc::CalcNode;
 use crate::values::{Auto, CSSFloat, Either, IsAuto, Normal};
@@ -99,6 +100,16 @@ impl FontBaseSize {
 }
 
 impl FontRelativeLength {
+    /// Return true if this is a zero value.
+    fn is_zero(&self) -> bool {
+        match *self {
+            FontRelativeLength::Em(v) |
+            FontRelativeLength::Ex(v) |
+            FontRelativeLength::Ch(v) |
+            FontRelativeLength::Rem(v) => v == 0.0,
+        }
+    }
+
     /// Computes the font-relative length.
     pub fn to_computed_value(&self, context: &Context, base_size: FontBaseSize) -> CSSPixelLength {
         use std::f32;
@@ -232,6 +243,16 @@ pub enum ViewportPercentageLength {
 }
 
 impl ViewportPercentageLength {
+    /// Return true if this is a zero value.
+    fn is_zero(&self) -> bool {
+        match *self {
+            ViewportPercentageLength::Vw(v) |
+            ViewportPercentageLength::Vh(v) |
+            ViewportPercentageLength::Vmin(v) |
+            ViewportPercentageLength::Vmax(v) => v == 0.0,
+        }
+    }
+
     /// Computes the given viewport-relative length for the given viewport size.
     pub fn to_computed_value(&self, viewport_size: Size2D<Au>) -> CSSPixelLength {
         let (factor, length) = match *self {
@@ -487,6 +508,18 @@ impl NoCalcLength {
 
 impl SpecifiedValueInfo for NoCalcLength {}
 
+impl IsZeroLength for NoCalcLength {
+    #[inline]
+    fn is_zero_length(&self) -> bool {
+        match *self {
+            NoCalcLength::Absolute(v) => v.is_zero(),
+            NoCalcLength::FontRelative(v) => v.is_zero(),
+            NoCalcLength::ViewportPercentage(v) => v.is_zero(),
+            NoCalcLength::ServoCharacterWidth(v) => v.0 == 0,
+        }
+    }
+}
+
 /// An extension to `NoCalcLength` to parse `calc` expressions.
 /// This is commonly used for the `<length>` values.
 ///
@@ -841,6 +874,17 @@ impl LengthOrPercentage {
     }
 }
 
+impl IsZeroLength for LengthOrPercentage {
+    #[inline]
+    fn is_zero_length(&self) -> bool {
+        match *self {
+            LengthOrPercentage::Length(l) => l.is_zero_length(),
+            LengthOrPercentage::Percentage(p) => p.0 == 0.0,
+            LengthOrPercentage::Calc(_) => false,
+        }
+    }
+}
+
 /// Either a `<length>`, a `<percentage>`, or the `auto` keyword.
 #[allow(missing_docs)]
 #[derive(Clone, Debug, MallocSizeOf, PartialEq, SpecifiedValueInfo, ToCss)]
diff --git a/components/style/values/specified/transform.rs b/components/style/values/specified/transform.rs
index 51c38a30ba8..661f105e555 100644
--- a/components/style/values/specified/transform.rs
+++ b/components/style/values/specified/transform.rs
@@ -395,7 +395,7 @@ impl Parse for Translate {
         }
 
         // 'translate: <length-percentage> '
-        Ok(generic::Translate::TranslateX(tx))
+        Ok(generic::Translate::Translate(tx, specified::LengthOrPercentage::zero()))
     }
 }