Bug 1434802 - Output unquoted family name as a series of identifiers. r?emilio draft
authorXidorn Quan <me@upsuper.org>
Thu, 08 Feb 2018 21:13:49 +1100
changeset 752852 ddde77d2b32a6e8db43afd9e4c4817bfde1a66ba
parent 752462 c7bd405f883823adb54b9a4a2b97eaef4966e11f
push id98399
push userxquan@mozilla.com
push dateFri, 09 Feb 2018 01:25:08 +0000
reviewersemilio
bugs1434802
milestone60.0a1
Bug 1434802 - Output unquoted family name as a series of identifiers. r?emilio This implements what Gecko's nsStyleUtil::AppendEscapedCSSFontFamilyList does, which should bring us to Gecko's original behavior. MozReview-Commit-ID: 4d10VOUpTkD
servo/components/style/values/computed/font.rs
--- a/servo/components/style/values/computed/font.rs
+++ b/servo/components/style/values/computed/font.rs
@@ -281,38 +281,47 @@ pub struct FamilyName {
 impl ToCss for FamilyName {
     fn to_css<W>(&self, dest: &mut CssWriter<W>) -> fmt::Result where W: fmt::Write {
         match self.syntax {
             FamilyNameSyntax::Quoted => {
                 dest.write_char('"')?;
                 write!(CssStringWriter::new(dest), "{}", self.name)?;
                 dest.write_char('"')
             }
-            FamilyNameSyntax::Identifiers(ref serialization) => {
-                // Note that `serialization` is already escaped/
-                // serialized appropriately.
-                dest.write_str(&*serialization)
+            FamilyNameSyntax::Identifiers => {
+                let mut first = true;
+                for ident in self.name.to_string().split(' ') {
+                    if first {
+                        first = false;
+                    } else {
+                        dest.write_char(' ')?;
+                    }
+                    debug_assert!(!ident.is_empty(), "Family name with leading, \
+                                  trailing, or consecutive white spaces should \
+                                  have been marked quoted by the parser");
+                    serialize_identifier(ident, dest)?;
+                }
+                Ok(())
             }
         }
     }
 }
 
 #[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq)]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
 /// Font family names must either be given quoted as strings,
 /// or unquoted as a sequence of one or more identifiers.
 pub enum FamilyNameSyntax {
     /// The family name was specified in a quoted form, e.g. "Font Name"
     /// or 'Font Name'.
     Quoted,
 
     /// The family name was specified in an unquoted form as a sequence of
-    /// identifiers.  The `String` is the serialization of the sequence of
     /// identifiers.
-    Identifiers(String),
+    Identifiers,
 }
 
 #[derive(Clone, Debug, Eq, Hash, MallocSizeOf, PartialEq)]
 #[cfg_attr(feature = "servo", derive(Deserialize, Serialize))]
 /// A set of faces that vary in weight, width or slope.
 pub enum SingleFontFamily {
     /// The name of a font family of choice.
     FamilyName(FamilyName),
@@ -401,37 +410,39 @@ impl SingleFontFamily {
             "inherit" => css_wide_keyword = true,
             "initial" => css_wide_keyword = true,
             "unset" => css_wide_keyword = true,
             "default" => css_wide_keyword = true,
             _ => {}
         }
 
         let mut value = first_ident.as_ref().to_owned();
-        let mut serialization = String::new();
-        serialize_identifier(&first_ident, &mut serialization).unwrap();
 
         // These keywords are not allowed by themselves.
         // The only way this value can be valid with with another keyword.
         if css_wide_keyword {
             let ident = input.expect_ident()?;
             value.push(' ');
             value.push_str(&ident);
-            serialization.push(' ');
-            serialize_identifier(&ident, &mut serialization).unwrap();
         }
         while let Ok(ident) = input.try(|i| i.expect_ident_cloned()) {
             value.push(' ');
             value.push_str(&ident);
-            serialization.push(' ');
-            serialize_identifier(&ident, &mut serialization).unwrap();
         }
+        let syntax = if value.starts_with(' ') || value.ends_with(' ') || value.contains("  ") {
+            // For font family names which contains special white spaces
+            // it is tricky to serialize as identifiers correctly. Just
+            // mark them quoted so we don't need to worry about them
+            // in serialization code.
+            FamilyNameSyntax::Quoted
+        } else {
+            FamilyNameSyntax::Identifiers
+        };
         Ok(SingleFontFamily::FamilyName(FamilyName {
-            name: Atom::from(value),
-            syntax: FamilyNameSyntax::Identifiers(serialization),
+            name: Atom::from(value), syntax
         }))
     }
 
     #[cfg(feature = "gecko")]
     /// Return the generic ID for a given generic font name
     pub fn generic(name: &Atom) -> (structs::FontFamilyType, u8) {
         use gecko_bindings::structs::FontFamilyType;
         if *name == atom!("serif") {
@@ -466,21 +477,19 @@ impl SingleFontFamily {
             FontFamilyType::eFamily_sans_serif => SingleFontFamily::Generic(atom!("sans-serif")),
             FontFamilyType::eFamily_serif => SingleFontFamily::Generic(atom!("serif")),
             FontFamilyType::eFamily_monospace => SingleFontFamily::Generic(atom!("monospace")),
             FontFamilyType::eFamily_cursive => SingleFontFamily::Generic(atom!("cursive")),
             FontFamilyType::eFamily_fantasy => SingleFontFamily::Generic(atom!("fantasy")),
             FontFamilyType::eFamily_moz_fixed => SingleFontFamily::Generic(Atom::from("-moz-fixed")),
             FontFamilyType::eFamily_named => {
                 let name = Atom::from(&*family.mName);
-                let mut serialization = String::new();
-                serialize_identifier(&name.to_string(), &mut serialization).unwrap();
                 SingleFontFamily::FamilyName(FamilyName {
                     name: name.clone(),
-                    syntax: FamilyNameSyntax::Identifiers(serialization),
+                    syntax: FamilyNameSyntax::Identifiers,
                 })
             },
             FontFamilyType::eFamily_named_quoted => SingleFontFamily::FamilyName(FamilyName {
                 name: (&*family.mName).into(),
                 syntax: FamilyNameSyntax::Quoted,
             }),
             _ => panic!("Found unexpected font FontFamilyType"),
         }