style: Remove the stupid space serialization on font-settings.

components/style/gecko/rules.rs

@@ -58,7 +58,7 @@ impl ToNsCssValue for font_feature_settings::T {
                 nscssvalue.set_pair_list(tags.into_iter().map(|entry| {
                     let mut feature = nsCSSValue::null();
                     let mut raw = [0u8; 4];
-                    (&mut raw[..]).write_u32::<BigEndian>(entry.tag).unwrap();
+                    (&mut raw[..]).write_u32::<BigEndian>(entry.tag.0).unwrap();
                     feature.set_string(str::from_utf8(&raw).unwrap());
 
                     let mut index = nsCSSValue::null();

components/style/properties/gecko.mako.rs

@@ -1444,7 +1444,7 @@ impl Clone for ${style_struct.gecko_struct_name} {
                 unsafe { current_settings.set_len_pod(other_settings.len() as u32) };
 
                 for (current, other) in current_settings.iter_mut().zip(other_settings) {
-                    current.mTag = other.tag;
+                    current.mTag = other.tag.0;
                     current.mValue = other.value.0;
                 }
             }
@@ -1470,7 +1470,7 @@ impl Clone for ${style_struct.gecko_struct_name} {
     }
 
     pub fn clone_${ident}(&self) -> longhands::${ident}::computed_value::T {
-        use values::generics::{FontSettings, FontSettingTag, ${tag_type}} ;
+        use values::generics::{FontSettings, FontTag, FontSettingTag, ${tag_type}} ;
 
         if self.gecko.mFont.${gecko_ffi_name}.len() == 0 {
             FontSettings::Normal
@@ -1478,7 +1478,7 @@ impl Clone for ${style_struct.gecko_struct_name} {
             FontSettings::Tag(
                 self.gecko.mFont.${gecko_ffi_name}.iter().map(|gecko_font_setting| {
                     FontSettingTag {
-                        tag: gecko_font_setting.mTag,
+                        tag: FontTag(gecko_font_setting.mTag),
                         value: ${tag_type}(gecko_font_setting.mValue),
                     }
                 }).collect()

components/style/properties/helpers/animated_properties.mako.rs

@@ -54,6 +54,7 @@ use values::distance::{ComputeSquaredDistance, SquaredDistance};
 #[cfg(feature = "gecko")] use values::generics::FontSettings as GenericFontSettings;
 #[cfg(feature = "gecko")] use values::generics::FontSettingTag as GenericFontSettingTag;
 #[cfg(feature = "gecko")] use values::generics::FontSettingTagFloat;
+#[cfg(feature = "gecko")] use values::generics::FontTag;
 use values::generics::NonNegative;
 use values::generics::effects::Filter;
 use values::generics::position as generic_position;
@@ -878,7 +879,7 @@ type FontSettingTag = GenericFontSettingTag<FontSettingTagFloat>;
 struct FontSettingTagIterState<'a> {
     tags: Vec<(&'a FontSettingTag)>,
     index: usize,
-    prev_tag: u32,
+    prev_tag: FontTag,
 }
 
 #[cfg(feature = "gecko")]
@@ -887,7 +888,7 @@ impl<'a> FontSettingTagIterState<'a> {
         FontSettingTagIterState {
             index: tags.len(),
             tags,
-            prev_tag: 0,
+            prev_tag: FontTag(0),
         }
     }
 }
@@ -941,7 +942,7 @@ impl<'a> FontSettingTagIter<'a> {
             fn as_new_sorted_tags(tags: &Vec<FontSettingTag>) -> Vec<(&FontSettingTag)> {
                 use std::iter::FromIterator;
                 let mut sorted_tags: Vec<(&FontSettingTag)> = Vec::from_iter(tags.iter());
-                sorted_tags.sort_by_key(|k| k.tag);
+                sorted_tags.sort_by_key(|k| k.tag.0);
                 sorted_tags
             };
 

components/style/values/generics/mod.rs

@@ -128,23 +128,37 @@ impl Parse for CounterStyleOrNone {
     }
 }
 
-/// A settings tag, defined by a four-character tag and a setting value
+/// A font four-character tag, represented as a u32 for convenience.
 ///
-/// For font-feature-settings, this is a tag and an integer,
+/// See:
-/// for font-variation-settings this is a tag and a float
+///   https://drafts.csswg.org/css-fonts-4/#font-variation-settings-def
-#[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue)]
+///   https://drafts.csswg.org/css-fonts-4/#descdef-font-face-font-feature-settings
-pub struct FontSettingTag<T> {
+///
-    /// A four-character tag, packed into a u32 (one byte per character)
+#[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue)]
-    pub tag: u32,
+pub struct FontTag(pub u32);
-    /// The value
+
-    pub value: T,
+impl Parse for FontTag {
+    fn parse<'i, 't>(
+        _context: &ParserContext,
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Self, ParseError<'i>> {
+        use byteorder::{ReadBytesExt, BigEndian};
+        use std::io::Cursor;
+
+        let location = input.current_source_location();
+        let tag = input.expect_string()?;
+
+        // allowed strings of length 4 containing chars: <U+20, U+7E>
+        if tag.len() != 4 || tag.as_bytes().iter().any(|c| *c < b' ' || *c > b'~') {
+            return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError))
+        }
+
+        let mut raw = Cursor::new(tag.as_bytes());
+        Ok(FontTag(raw.read_u32::<BigEndian>().unwrap()))
+    }
 }
 
-impl<T> OneOrMoreSeparated for FontSettingTag<T> {
+impl ToCss for FontTag {
-    type S = Comma;
-}
-
-impl<T: ToCss> ToCss for FontSettingTag<T> {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
     where
         W: Write,
@@ -153,44 +167,44 @@ impl<T: ToCss> ToCss for FontSettingTag<T> {
         use std::str;
 
         let mut raw = [0u8; 4];
-        BigEndian::write_u32(&mut raw, self.tag);
+        BigEndian::write_u32(&mut raw, self.0);
-        str::from_utf8(&raw).unwrap_or_default().to_css(dest)?;
+        str::from_utf8(&raw).unwrap_or_default().to_css(dest)
-
-        self.value.to_css(dest)
     }
 }
 
+/// A settings tag, defined by a four-character tag and a setting value
+///
+/// For font-feature-settings, this is a tag and an integer, for
+/// font-variation-settings this is a tag and a float.
+#[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue, ToCss)]
+pub struct FontSettingTag<T> {
+    /// A four-character tag, packed into a u32 (one byte per character)
+    pub tag: FontTag,
+    /// The actual value.
+    pub value: T,
+}
+
+impl<T> OneOrMoreSeparated for FontSettingTag<T> {
+    type S = Comma;
+}
+
 impl<T: Parse> Parse for FontSettingTag<T> {
     /// <https://www.w3.org/TR/css-fonts-3/#propdef-font-feature-settings>
     /// <https://drafts.csswg.org/css-fonts-4/#low-level-font-variation->
     /// settings-control-the-font-variation-settings-property
     /// <string> [ on | off | <integer> ]
     /// <string> <number>
-    fn parse<'i, 't>(context: &ParserContext, input: &mut Parser<'i, 't>) -> Result<Self, ParseError<'i>> {
+    fn parse<'i, 't>(
-        use byteorder::{ReadBytesExt, BigEndian};
+        context: &ParserContext,
-        use std::io::Cursor;
+        input: &mut Parser<'i, 't>,
+    ) -> Result<Self, ParseError<'i>> {
+        let tag = FontTag::parse(context, input)?;
+        let value = T::parse(context, input)?;
 
-        let u_tag;
+        Ok(Self { tag, value })
-        {
-            let location = input.current_source_location();
-            let tag = input.expect_string()?;
-
-            // allowed strings of length 4 containing chars: <U+20, U+7E>
-            if tag.len() != 4 ||
-               tag.chars().any(|c| c < ' ' || c > '~')
-            {
-                return Err(location.new_custom_error(StyleParseErrorKind::UnspecifiedError))
-            }
-
-            let mut raw = Cursor::new(tag.as_bytes());
-            u_tag = raw.read_u32::<BigEndian>().unwrap();
-        }
-
-        Ok(FontSettingTag { tag: u_tag, value: T::parse(context, input)? })
     }
 }
 
-
 /// A font settings value for font-variation-settings or font-feature-settings
 #[derive(Clone, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue, ToCss)]
 pub enum FontSettings<T> {
@@ -220,18 +234,17 @@ impl<T: Parse> Parse for FontSettings<T> {
 
 /// An integer that can also parse "on" and "off",
 /// for font-feature-settings
-///
-/// Do not use this type anywhere except within FontSettings
-/// because it serializes with the preceding space
 #[derive(Clone, Copy, Debug, Eq, MallocSizeOf, PartialEq, ToComputedValue)]
 pub struct FontSettingTagInt(pub u32);
 
-/// A number value to be used for font-variation-settings
+/// A number value to be used for font-variation-settings.
 ///
-/// Do not use this type anywhere except within FontSettings
+/// FIXME(emilio): The spec only says <integer>, so we should be able to reuse
-/// because it serializes with the preceding space
+/// the other code:
+///
+/// https://drafts.csswg.org/css-fonts-4/#propdef-font-variation-settings
 #[cfg_attr(feature = "gecko", derive(Animate, ComputeSquaredDistance))]
-#[derive(Clone, Debug, MallocSizeOf, PartialEq, ToComputedValue)]
+#[derive(Clone, Debug, MallocSizeOf, PartialEq, ToComputedValue, ToCss)]
 pub struct FontSettingTagFloat(pub f32);
 
 impl ToCss for FontSettingTagInt {
@@ -241,13 +254,8 @@ impl ToCss for FontSettingTagInt {
     {
         match self.0 {
             1 => Ok(()),
-            0 => dest.write_str(" off"),
+            0 => dest.write_str("off"),
-            x => {
+            x => x.to_css(dest),
-                // FIXME(emilio): Why in the world does this serialize the space
-                // itself?
-                dest.write_char(' ')?;
-                x.to_css(dest)
-            }
         }
     }
 }
@@ -284,16 +292,6 @@ impl Parse for FontSettingTagFloat {
     }
 }
 
-impl ToCss for FontSettingTagFloat {
-    fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result
-    where
-        W: Write,
-    {
-        dest.write_str(" ")?;
-        self.0.to_css(dest)
-    }
-}
-
 /// A wrapper of Non-negative values.
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
 #[derive(Animate, Clone, ComputeSquaredDistance, Copy, Debug, MallocSizeOf)]