Use generics for text spacing properties

components/layout/text.rs

@@ -166,8 +166,8 @@ impl TextRunScanner {
                     white_space::T::pre_line => CompressionMode::CompressWhitespace,
                 };
                 text_transform = inherited_text_style.text_transform;
-                letter_spacing = inherited_text_style.letter_spacing.0;
+                letter_spacing = inherited_text_style.letter_spacing;
-                word_spacing = inherited_text_style.word_spacing.0
+                word_spacing = inherited_text_style.word_spacing.value().cloned()
                                .map(|lop| lop.to_hash_key())
                                .unwrap_or((Au(0), NotNaN::new(0.0).unwrap()));
                 text_rendering = inherited_text_style.text_rendering;
@@ -289,8 +289,8 @@ impl TextRunScanner {
             // example, `finally` with a wide `letter-spacing` renders as `f i n a l l y` and not
             // `fi n a l l y`.
             let mut flags = ShapingFlags::empty();
-            match letter_spacing {
+            match letter_spacing.value() {
-                Some(Au(0)) | None => {}
+                Some(&Au(0)) | None => {}
                 Some(_) => flags.insert(IGNORE_LIGATURES_SHAPING_FLAG),
             }
             if text_rendering == text_rendering::T::optimizespeed {
@@ -301,7 +301,7 @@ impl TextRunScanner {
                 flags.insert(KEEP_ALL_FLAG);
             }
             let options = ShapingOptions {
-                letter_spacing: letter_spacing,
+                letter_spacing: letter_spacing.value().cloned(),
                 word_spacing: word_spacing,
                 script: Script::Common,
                 flags: flags,

components/style/properties/gecko.mako.rs

@@ -3664,35 +3664,37 @@ fn static_assert() {
     <%call expr="impl_coord_copy('line_height', 'mLineHeight')"></%call>
 
     pub fn set_letter_spacing(&mut self, v: longhands::letter_spacing::computed_value::T) {
-        match v.0 {
+        use values::generics::text::Spacing;
-            Some(au) => self.gecko.mLetterSpacing.set(au),
+        match v {
-            None => self.gecko.mLetterSpacing.set_value(CoordDataValue::Normal)
+            Spacing::Value(value) => self.gecko.mLetterSpacing.set(value),
+            Spacing::Normal => self.gecko.mLetterSpacing.set_value(CoordDataValue::Normal)
         }
     }
 
     pub fn clone_letter_spacing(&self) -> longhands::letter_spacing::computed_value::T {
-        use properties::longhands::letter_spacing::computed_value::T;
+        use values::generics::text::Spacing;
         debug_assert!(
             matches!(self.gecko.mLetterSpacing.as_value(),
                      CoordDataValue::Normal |
                      CoordDataValue::Coord(_)),
             "Unexpected computed value for letter-spacing");
-        T(Au::from_gecko_style_coord(&self.gecko.mLetterSpacing))
+        Au::from_gecko_style_coord(&self.gecko.mLetterSpacing).map_or(Spacing::Normal, Spacing::Value)
     }
 
     <%call expr="impl_coord_copy('letter_spacing', 'mLetterSpacing')"></%call>
 
     pub fn set_word_spacing(&mut self, v: longhands::word_spacing::computed_value::T) {
-        match v.0 {
+        use values::generics::text::Spacing;
-            Some(lop) => self.gecko.mWordSpacing.set(lop),
+        match v {
+            Spacing::Value(lop) => self.gecko.mWordSpacing.set(lop),
             // https://drafts.csswg.org/css-text-3/#valdef-word-spacing-normal
-            None => self.gecko.mWordSpacing.set_value(CoordDataValue::Coord(0)),
+            Spacing::Normal => self.gecko.mWordSpacing.set_value(CoordDataValue::Coord(0)),
         }
     }
 
     pub fn clone_word_spacing(&self) -> longhands::word_spacing::computed_value::T {
-        use properties::longhands::word_spacing::computed_value::T;
         use values::computed::LengthOrPercentage;
+        use values::generics::text::Spacing;
         debug_assert!(
             matches!(self.gecko.mWordSpacing.as_value(),
                      CoordDataValue::Normal |
@@ -3700,7 +3702,7 @@ fn static_assert() {
                      CoordDataValue::Percent(_) |
                      CoordDataValue::Calc(_)),
             "Unexpected computed value for word-spacing");
-        T(LengthOrPercentage::from_gecko_style_coord(&self.gecko.mWordSpacing))
+        LengthOrPercentage::from_gecko_style_coord(&self.gecko.mWordSpacing).map_or(Spacing::Normal, Spacing::Value)
     }
 
     <%call expr="impl_coord_copy('word_spacing', 'mWordSpacing')"></%call>

components/style/properties/longhand/inherited_text.mako.rs

@@ -258,157 +258,17 @@ ${helpers.single_keyword("text-align-last",
     % endif
 </%helpers:longhand>
 
-<%helpers:longhand name="letter-spacing" animation_value_type="ComputedValue"
+${helpers.predefined_type("letter-spacing",
-                   spec="https://drafts.csswg.org/css-text/#propdef-letter-spacing">
+                          "LetterSpacing",
-    use std::fmt;
+                          "computed::LetterSpacing::normal()",
-    use style_traits::ToCss;
+                          animation_value_type="ComputedValue",
-    use values::specified::AllowQuirks;
+                          spec="https://drafts.csswg.org/css-text/#propdef-letter-spacing")}
 
-    #[derive(Clone, Debug, HasViewportPercentage, PartialEq)]
+${helpers.predefined_type("word-spacing",
-    #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+                          "WordSpacing",
-    pub enum SpecifiedValue {
+                          "computed::WordSpacing::normal()",
-        Normal,
+                          animation_value_type="ComputedValue",
-        Specified(specified::Length),
+                          spec="https://drafts.csswg.org/css-text/#propdef-word-spacing")}
-    }
-
-    impl ToCss for SpecifiedValue {
-        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            match *self {
-                SpecifiedValue::Normal => dest.write_str("normal"),
-                SpecifiedValue::Specified(ref l) => l.to_css(dest),
-            }
-        }
-    }
-
-    pub mod computed_value {
-        use app_units::Au;
-        use properties::animated_properties::Animatable;
-
-        #[derive(Debug, Clone, PartialEq)]
-        #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-        pub struct T(pub Option<Au>);
-
-        ${helpers.impl_animatable_for_option_tuple('Au(0)')}
-    }
-
-    impl ToCss for computed_value::T {
-        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            match self.0 {
-                None => dest.write_str("normal"),
-                Some(l) => l.to_css(dest),
-            }
-        }
-    }
-
-    #[inline]
-    pub fn get_initial_value() -> computed_value::T {
-        computed_value::T(None)
-    }
-
-    impl ToComputedValue for SpecifiedValue {
-        type ComputedValue = computed_value::T;
-
-        #[inline]
-        fn to_computed_value(&self, context: &Context) -> computed_value::T {
-            match *self {
-                SpecifiedValue::Normal => computed_value::T(None),
-                SpecifiedValue::Specified(ref l) =>
-                    computed_value::T(Some(l.to_computed_value(context)))
-            }
-        }
-
-        #[inline]
-        fn from_computed_value(computed: &computed_value::T) -> Self {
-            computed.0.map(|ref au| {
-                SpecifiedValue::Specified(ToComputedValue::from_computed_value(au))
-            }).unwrap_or(SpecifiedValue::Normal)
-        }
-    }
-
-    pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
-        if input.try(|input| input.expect_ident_matching("normal")).is_ok() {
-            Ok(SpecifiedValue::Normal)
-        } else {
-            specified::Length::parse_quirky(context, input, AllowQuirks::Yes).map(SpecifiedValue::Specified)
-        }
-    }
-</%helpers:longhand>
-
-<%helpers:longhand name="word-spacing" animation_value_type="ComputedValue"
-                   spec="https://drafts.csswg.org/css-text/#propdef-word-spacing">
-    use std::fmt;
-    use style_traits::ToCss;
-    use values::specified::AllowQuirks;
-
-    #[derive(Clone, Debug, HasViewportPercentage, PartialEq)]
-    #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-    pub enum SpecifiedValue {
-        Normal,
-        Specified(specified::LengthOrPercentage),
-    }
-
-    impl ToCss for SpecifiedValue {
-        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            match *self {
-                SpecifiedValue::Normal => dest.write_str("normal"),
-                SpecifiedValue::Specified(ref l) => l.to_css(dest),
-            }
-        }
-    }
-
-    pub mod computed_value {
-        use properties::animated_properties::Animatable;
-        use values::computed::LengthOrPercentage;
-        #[derive(Debug, Clone, PartialEq)]
-        #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
-        pub struct T(pub Option<LengthOrPercentage>);
-
-        ${helpers.impl_animatable_for_option_tuple('LengthOrPercentage::zero()')}
-    }
-
-    impl ToCss for computed_value::T {
-        fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-            match self.0 {
-                None => dest.write_str("normal"),
-                Some(l) => l.to_css(dest),
-            }
-        }
-    }
-
-    #[inline]
-    pub fn get_initial_value() -> computed_value::T {
-        computed_value::T(None)
-    }
-
-    impl ToComputedValue for SpecifiedValue {
-        type ComputedValue = computed_value::T;
-
-        #[inline]
-        fn to_computed_value(&self, context: &Context) -> computed_value::T {
-            match *self {
-                SpecifiedValue::Normal => computed_value::T(None),
-                SpecifiedValue::Specified(ref l) =>
-                    computed_value::T(Some(l.to_computed_value(context))),
-            }
-        }
-
-        #[inline]
-        fn from_computed_value(computed: &computed_value::T) -> Self {
-            computed.0.map(|ref lop| {
-                SpecifiedValue::Specified(ToComputedValue::from_computed_value(lop))
-            }).unwrap_or(SpecifiedValue::Normal)
-        }
-    }
-
-    pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
-        if input.try(|input| input.expect_ident_matching("normal")).is_ok() {
-            Ok(SpecifiedValue::Normal)
-        } else {
-            specified::LengthOrPercentage::parse_quirky(context, input, AllowQuirks::Yes)
-                                          .map(SpecifiedValue::Specified)
-        }
-    }
-</%helpers:longhand>
 
 <%helpers:longhand name="-servo-text-decorations-in-effect"
                    derived_from="display text-decoration"

components/style/values/computed/length.rs

@@ -222,6 +222,13 @@ pub enum LengthOrPercentage {
     Calc(CalcLengthOrPercentage),
 }
 
+impl From<Au> for LengthOrPercentage {
+    #[inline]
+    fn from(length: Au) -> Self {
+        LengthOrPercentage::Length(length)
+    }
+}
+
 impl LengthOrPercentage {
     #[inline]
     #[allow(missing_docs)]

components/style/values/computed/mod.rs

@@ -38,7 +38,7 @@ pub use self::length::{CalcLengthOrPercentage, Length, LengthOrNumber, LengthOrP
 pub use self::length::{LengthOrPercentageOrAutoOrContent, LengthOrPercentageOrNone, LengthOrNone};
 pub use self::length::{MaxLength, MozLength};
 pub use self::position::Position;
-pub use self::text::LineHeight;
+pub use self::text::{LetterSpacing, LineHeight, WordSpacing};
 pub use self::transform::TransformOrigin;
 
 pub mod background;

components/style/values/computed/text.rs

@@ -7,7 +7,14 @@
 use app_units::Au;
 use properties::animated_properties::Animatable;
 use values::CSSFloat;
-use values::generics::text::LineHeight as GenericLineHeight;
+use values::computed::length::{Length, LengthOrPercentage};
+use values::generics::text::{LineHeight as GenericLineHeight, Spacing};
+
+/// A computed value for the `letter-spacing` property.
+pub type LetterSpacing = Spacing<Length>;
+
+/// A computed value for the `word-spacing` property.
+pub type WordSpacing = Spacing<LengthOrPercentage>;
 
 /// A computed value for the `line-height` property.
 pub type LineHeight = GenericLineHeight<CSSFloat, Au>;

components/style/values/generics/text.rs

@@ -4,9 +4,91 @@
 
 //! Generic types for text properties.
 
+use app_units::Au;
+use cssparser::Parser;
+use parser::ParserContext;
+use properties::animated_properties::Animatable;
 use std::fmt;
 use style_traits::ToCss;
 
+/// A generic spacing value for the `letter-spacing` and `word-spacing` properties.alloc
+#[cfg_attr(feature = "servo", derive(HeapSizeOf))]
+#[derive(Clone, Copy, Debug, HasViewportPercentage, PartialEq, ToComputedValue)]
+pub enum Spacing<Value> {
+    /// `normal`
+    Normal,
+    /// `<value>`
+    Value(Value),
+}
+
+impl<Value> Spacing<Value> {
+    /// Returns `normal`.
+    #[inline]
+    pub fn normal() -> Self {
+        Spacing::Normal
+    }
+
+    /// Parses.
+    #[inline]
+    pub fn parse_with<F>(
+        context: &ParserContext,
+        input: &mut Parser,
+        parse: F)
+        -> Result<Self, ()>
+        where F: FnOnce(&ParserContext, &mut Parser) -> Result<Value, ()>
+    {
+        if input.try(|i| i.expect_ident_matching("normal")).is_ok() {
+            return Ok(Spacing::Normal);
+        }
+        parse(context, input).map(Spacing::Value)
+    }
+
+    /// Returns the spacing value, if not `normal`.
+    #[inline]
+    pub fn value(&self) -> Option<&Value> {
+        match *self {
+            Spacing::Normal => None,
+            Spacing::Value(ref value) => Some(value),
+        }
+    }
+}
+
+impl<Value> Animatable for Spacing<Value>
+    where Value: Animatable + From<Au>,
+{
+    #[inline]
+    fn add_weighted(&self, other: &Self, self_portion: f64, other_portion: f64) -> Result<Self, ()> {
+        if let (&Spacing::Normal, &Spacing::Normal) = (self, other) {
+            return Ok(Spacing::Normal);
+        }
+        let zero = Value::from(Au(0));
+        let this = self.value().unwrap_or(&zero);
+        let other = other.value().unwrap_or(&zero);
+        this.add_weighted(other, self_portion, other_portion).map(Spacing::Value)
+    }
+
+    #[inline]
+    fn compute_distance(&self, other: &Self) -> Result<f64, ()> {
+        let zero = Value::from(Au(0));
+        let this = self.value().unwrap_or(&zero);
+        let other = other.value().unwrap_or(&zero);
+        this.compute_distance(other)
+    }
+}
+
+impl<Value> ToCss for Spacing<Value>
+    where Value: ToCss,
+{
+    fn to_css<W>(&self, dest: &mut W) -> fmt::Result
+        where W: fmt::Write
+    {
+        match *self {
+            Spacing::Normal => dest.write_str("normal"),
+            Spacing::Value(ref value) => value.to_css(dest),
+        }
+    }
+}
+
 /// A generic value for the `line-height` property.
 #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
 #[derive(Clone, Copy, Debug, HasViewportPercentage, PartialEq)]

components/style/values/specified/mod.rs

@@ -44,7 +44,7 @@ pub use self::length::{Percentage, LengthOrNone, LengthOrNumber, LengthOrPercent
 pub use self::length::{LengthOrPercentageOrNone, LengthOrPercentageOrAutoOrContent, NoCalcLength};
 pub use self::length::{MaxLength, MozLength};
 pub use self::position::{Position, PositionComponent};
-pub use self::text::LineHeight;
+pub use self::text::{LetterSpacing, LineHeight, WordSpacing};
 pub use self::transform::TransformOrigin;
 
 #[cfg(feature = "gecko")]

components/style/values/specified/text.rs

@@ -9,13 +9,35 @@ use parser::{Parse, ParserContext};
 use std::ascii::AsciiExt;
 use values::computed::{Context, ToComputedValue};
 use values::computed::text::LineHeight as ComputedLineHeight;
-use values::generics::text::LineHeight as GenericLineHeight;
+use values::generics::text::{LineHeight as GenericLineHeight, Spacing};
-use values::specified::Number;
+use values::specified::{AllowQuirks, Number};
 use values::specified::length::{FontRelativeLength, Length, LengthOrPercentage, NoCalcLength};
 
+/// A specified value for the `letter-spacing` property.
+pub type LetterSpacing = Spacing<Length>;
+
+/// A specified value for the `word-spacing` property.
+pub type WordSpacing = Spacing<LengthOrPercentage>;
+
 /// A specified value for the `line-height` property.
 pub type LineHeight = GenericLineHeight<Number, LengthOrPercentage>;
 
+impl Parse for LetterSpacing {
+    fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
+        Spacing::parse_with(context, input, |c, i| {
+            Length::parse_quirky(c, i, AllowQuirks::Yes)
+        })
+    }
+}
+
+impl Parse for WordSpacing {
+    fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
+        Spacing::parse_with(context, input, |c, i| {
+            LengthOrPercentage::parse_quirky(c, i, AllowQuirks::Yes)
+        })
+    }
+}
+
 impl Parse for LineHeight {
     fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
         if let Ok(number) = input.try(|i| Number::parse_non_negative(context, i)) {

tests/unit/style/parsing/inherited_text.rs

@@ -3,27 +3,27 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 use parsing::parse;
+use style::values::generics::text::Spacing;
 
 #[test]
 fn negative_letter_spacing_should_parse_properly() {
     use style::properties::longhands::letter_spacing;
-    use style::properties::longhands::letter_spacing::SpecifiedValue;
     use style::values::specified::length::{Length, NoCalcLength, FontRelativeLength};
 
     let negative_value = parse_longhand!(letter_spacing, "-0.5em");
-    let expected = SpecifiedValue::Specified(Length::NoCalc(NoCalcLength::FontRelative(FontRelativeLength::Em(-0.5))));
+    let expected = Spacing::Value(Length::NoCalc(NoCalcLength::FontRelative(FontRelativeLength::Em(-0.5))));
     assert_eq!(negative_value, expected);
 }
 
 #[test]
 fn negative_word_spacing_should_parse_properly() {
     use style::properties::longhands::word_spacing;
-    use style::properties::longhands::word_spacing::SpecifiedValue;
     use style::values::specified::length::{NoCalcLength, LengthOrPercentage, FontRelativeLength};
 
     let negative_value = parse_longhand!(word_spacing, "-0.5em");
-    let expected = SpecifiedValue::Specified(LengthOrPercentage::Length(NoCalcLength::FontRelative(
+    let expected = Spacing::Value(LengthOrPercentage::Length(
-                                             FontRelativeLength::Em(-0.5))));
+        NoCalcLength::FontRelative(FontRelativeLength::Em(-0.5))
+    ));
     assert_eq!(negative_value, expected);
 }