Auto merge of #16194 - canaltinova:font-family, r=Manishearth

stylo: Serialize unquoted font-family without quote

<!-- Please describe your changes on the following line: -->
Reviewed by Manishearth

---
<!-- Thank you for contributing to Servo! Please replace each `[ ]` by `[X]` when the step is complete, and replace `__` with appropriate data: -->
- [X] `./mach build -d` does not report any errors
- [X] `./mach test-tidy` does not report any errors
- [X] These changes fix [Bug 1351262](https://bugzilla.mozilla.org/show_bug.cgi?id=1351262)

<!-- Also, please make sure that "Allow edits from maintainers" checkbox is checked, so that we can help you if you get stuck somewhere along the way.-->

<!-- Pull requests that do not address these steps are welcome, but they will require additional verification as part of the review process. -->

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/16194)
<!-- Reviewable:end -->

8 files changed, 53 insertions, 24 deletions

diff --git a/components/gfx/font_cache_thread.rs b/components/gfx/font_cache_thread.rs
index 4a25a50521c..756d0744843 100644
--- a/components/gfx/font_cache_thread.rs
+++ b/components/gfx/font_cache_thread.rs
@@ -269,7 +269,7 @@ impl FontCache {
                 });
             }
             Source::Local(ref font) => {
-                let font_face_name = LowercaseString::new(&font.0);
+                let font_face_name = LowercaseString::new(&font.name);
                 let templates = &mut self.web_families.get_mut(&family_name).unwrap();
                 let mut found = false;
                 for_each_variation(&font_face_name, |path| {
@@ -464,7 +464,7 @@ impl FontCacheThread {
     }
 
     pub fn add_web_font(&self, family: FamilyName, sources: EffectiveSources, sender: IpcSender<()>) {
-        self.chan.send(Command::AddWebFont(LowercaseString::new(&family.0), sources, sender)).unwrap();
+        self.chan.send(Command::AddWebFont(LowercaseString::new(&family.name), sources, sender)).unwrap();
     }
 
     pub fn exit(&self) {
diff --git a/components/style/font_face.rs b/components/style/font_face.rs
index e585f15efaa..2b441ceb0d7 100644
--- a/components/style/font_face.rs
+++ b/components/style/font_face.rs
@@ -282,7 +282,10 @@ macro_rules! font_face_descriptors {
 font_face_descriptors! {
     mandatory descriptors = [
         /// The name of this font face
-        "font-family" family: FamilyName = FamilyName(atom!("")),
+        "font-family" family: FamilyName = FamilyName {
+            name: atom!(""),
+            quoted: true,
+        },
 
         /// The alternative sources for this font face.
         "src" sources: Vec<Source> = Vec::new(),
@@ -308,7 +311,10 @@ font_face_descriptors! {
 font_face_descriptors! {
     mandatory descriptors = [
         /// The name of this font face
-        "font-family" family: FamilyName = FamilyName(atom!("")),
+        "font-family" family: FamilyName = FamilyName {
+            name: atom!(""),
+            quoted: true,
+        },
 
         /// The alternative sources for this font face.
         "src" sources: Vec<Source> = Vec::new(),
diff --git a/components/style/gecko/rules.rs b/components/style/gecko/rules.rs
index f725a65a524..4b1a331ea21 100644
--- a/components/style/gecko/rules.rs
+++ b/components/style/gecko/rules.rs
@@ -17,7 +17,7 @@ pub type FontFaceRule = RefPtr<nsCSSFontFaceRule>;
 fn set_font_face_descriptors(descriptors: &mut CSSFontFaceDescriptors,
                              data: FontFaceData) {
     // font-family
-    descriptors.mFamily.set_string_from_atom(&data.family.0);
+    descriptors.mFamily.set_string_from_atom(&data.family.name);
 
     macro_rules! map_enum {
         ($target:ident = ($data:ident: $prop:ident) {
@@ -74,8 +74,8 @@ fn set_font_face_descriptors(descriptors: &mut CSSFontFaceDescriptors,
                     next!().set_font_format(&hint);
                 }
             }
-            Source::Local(ref name) => {
-                next!().set_local_font(&name.0);
+            Source::Local(ref family) => {
+                next!().set_local_font(&family.name);
             }
         }
     }
diff --git a/components/style/gecko_bindings/bindings.rs b/components/style/gecko_bindings/bindings.rs
index 77aaf994e99..c39a5fbe5c8 100644
--- a/components/style/gecko_bindings/bindings.rs
+++ b/components/style/gecko_bindings/bindings.rs
@@ -660,7 +660,8 @@ extern "C" {
 }
 extern "C" {
     pub fn Gecko_FontFamilyList_AppendNamed(aList: *mut FontFamilyList,
-                                            aName: *mut nsIAtom);
+                                            aName: *mut nsIAtom,
+                                            aQuoted: bool);
 }
 extern "C" {
     pub fn Gecko_FontFamilyList_AppendGeneric(list: *mut FontFamilyList,
diff --git a/components/style/properties/gecko.mako.rs b/components/style/properties/gecko.mako.rs
index ed6591d6cd6..78db1f0bd62 100644
--- a/components/style/properties/gecko.mako.rs
+++ b/components/style/properties/gecko.mako.rs
@@ -1257,8 +1257,8 @@ fn static_assert() {
 
         for family in &v.0 {
             match *family {
-                FontFamily::FamilyName(ref name) => {
-                    unsafe { Gecko_FontFamilyList_AppendNamed(list, name.0.as_ptr()); }
+                FontFamily::FamilyName(ref f) => {
+                    unsafe { Gecko_FontFamilyList_AppendNamed(list, f.name.as_ptr(), f.quoted); }
                 }
                 FontFamily::Generic(ref name) => {
                     let (family_type, generic) =
diff --git a/components/style/properties/longhand/font.mako.rs b/components/style/properties/longhand/font.mako.rs
index 0640a853a1b..abceff2dcd5 100644
--- a/components/style/properties/longhand/font.mako.rs
+++ b/components/style/properties/longhand/font.mako.rs
@@ -18,7 +18,7 @@
     no_viewport_percentage!(SpecifiedValue);
 
     pub mod computed_value {
-        use cssparser::{CssStringWriter, Parser};
+        use cssparser::{CssStringWriter, Parser, serialize_identifier};
         use std::fmt::{self, Write};
         use Atom;
         use style_traits::ToCss;
@@ -33,13 +33,16 @@
 
         #[derive(Debug, PartialEq, Eq, Clone, Hash)]
         #[cfg_attr(feature = "servo", derive(HeapSizeOf, Deserialize, Serialize))]
-        pub struct FamilyName(pub Atom);
+        pub struct FamilyName {
+            pub name: Atom,
+            pub quoted: bool,
+        }
 
         impl FontFamily {
             #[inline]
             pub fn atom(&self) -> &Atom {
                 match *self {
-                    FontFamily::FamilyName(ref name) => &name.0,
+                    FontFamily::FamilyName(ref family_name) => &family_name.name,
                     FontFamily::Generic(ref name) => name,
                 }
             }
@@ -70,13 +73,22 @@
                     "monospace" => return FontFamily::Generic(atom!("monospace")),
                     _ => {}
                 }
-                FontFamily::FamilyName(FamilyName(input))
+
+                // We don't know if it's quoted or not. So we set it to
+                // quoted by default.
+                FontFamily::FamilyName(FamilyName {
+                    name: input,
+                    quoted: true,
+                })
             }
 
             /// Parse a font-family value
             pub fn parse(input: &mut Parser) -> Result<Self, ()> {
                 if let Ok(value) = input.try(|input| input.expect_string()) {
-                    return Ok(FontFamily::FamilyName(FamilyName(Atom::from(&*value))))
+                    return Ok(FontFamily::FamilyName(FamilyName {
+                        name: Atom::from(&*value),
+                        quoted: true,
+                    }))
                 }
                 let first_ident = try!(input.expect_ident());
 
@@ -120,15 +132,22 @@
                     value.push_str(" ");
                     value.push_str(&ident);
                 }
-                Ok(FontFamily::FamilyName(FamilyName(Atom::from(value))))
+                Ok(FontFamily::FamilyName(FamilyName {
+                    name: Atom::from(value),
+                    quoted: false,
+                }))
             }
         }
 
         impl ToCss for FamilyName {
             fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
-                dest.write_char('"')?;
-                write!(CssStringWriter::new(dest), "{}", self.0)?;
-                dest.write_char('"')
+                if self.quoted {
+                    dest.write_char('"')?;
+                    write!(CssStringWriter::new(dest), "{}", self.name)?;
+                    dest.write_char('"')
+                } else {
+                    serialize_identifier(&*self.name.to_string(), dest)
+                }
             }
         }
 
diff --git a/tests/unit/gfx/font_cache_thread.rs b/tests/unit/gfx/font_cache_thread.rs
index 4c62c15ec34..1ce649f392c 100644
--- a/tests/unit/gfx/font_cache_thread.rs
+++ b/tests/unit/gfx/font_cache_thread.rs
@@ -12,8 +12,14 @@ fn test_local_web_font() {
     let (inp_chan, _) = ipc::channel().unwrap();
     let (out_chan, out_receiver) = ipc::channel().unwrap();
     let font_cache_thread = FontCacheThread::new(inp_chan, None);
-    let family_name = FamilyName(From::from("test family"));
-    let variant_name = FamilyName(From::from("test font face"));
+    let family_name = FamilyName {
+        name: From::from("test family"),
+        quoted: true,
+    };
+    let variant_name = FamilyName {
+        name: From::from("test font face"),
+        quoted: true,
+    };
     let font_face_rule = FontFaceData {
         family: family_name.clone(),
         sources: vec![Source::Local(variant_name)],
diff --git a/tests/wpt/metadata/cssom/serialize-values.html.ini b/tests/wpt/metadata/cssom/serialize-values.html.ini
index 07880ae3d38..431e522889a 100644
--- a/tests/wpt/metadata/cssom/serialize-values.html.ini
+++ b/tests/wpt/metadata/cssom/serialize-values.html.ini
@@ -16,9 +16,6 @@
   [content: attr(foo_bar)]
     expected: FAIL
 
-  [font-family: Arial]
-    expected: FAIL
-
   [list-style-type: decimal-leading-zero]
     expected: FAIL