Fix #2802: fix parsing of font-family in @font-face

3 files changed, 89 insertions, 54 deletions

diff --git a/src/components/style/font_face.rs b/src/components/style/font_face.rs
index 43a5cc65dca..f46ad635dcb 100644
--- a/src/components/style/font_face.rs
+++ b/src/components/style/font_face.rs
@@ -6,7 +6,7 @@ use cssparser::ast::*;
 use cssparser::parse_declaration_list;
 use errors::{ErrorLoggerIterator, log_css_error};
 use std::ascii::StrAsciiExt;
-use parsing_utils::one_component_value;
+use parsing_utils::BufferedIter;
 use stylesheets::{CSSRule, CSSFontFaceRule};
 use url::{Url, UrlParser};
 
@@ -59,14 +59,13 @@ pub fn parse_font_face_rule(rule: AtRule, parent_rules: &mut Vec<CSSRule>, base_
                 let name_lower = name.as_slice().to_ascii_lower();
                 match name_lower.as_slice() {
                     "font-family" => {
-                        // FIXME(#2802): Share code with the font-family parser.
-                        match one_component_value(value.as_slice()) {
-                            Ok(&String(ref string_value)) => {
-                                maybe_family = Some(string_value.clone());
+                        let iter = &mut BufferedIter::new(value.as_slice().skip_whitespace());
+                        match ::properties::longhands::font_family::parse_one_family(iter) {
+                            Ok(::properties::computed_values::font_family::FamilyName(name)) => {
+                                maybe_family = Some(name);
                             },
-                            _ => {
-                                log_css_error(location, format!("Unsupported font-family string {:s}", name).as_slice());
-                            }
+                            // This also includes generic family names:
+                            _ => log_css_error(location, "Invalid font-family in @font-face"),
                         }
                     },
                     "src" => {
diff --git a/src/components/style/parsing_utils.rs b/src/components/style/parsing_utils.rs
index fef2506bfe2..413cad78601 100644
--- a/src/components/style/parsing_utils.rs
+++ b/src/components/style/parsing_utils.rs
@@ -4,7 +4,7 @@
 
 
 use std::ascii::StrAsciiExt;
-use cssparser::ast::{ComponentValue, Ident, SkipWhitespaceIterable};
+use cssparser::ast::{ComponentValue, Ident, SkipWhitespaceIterable, SkipWhitespaceIterator};
 
 
 pub fn one_component_value<'a>(input: &'a [ComponentValue]) -> Result<&'a ComponentValue, ()> {
@@ -22,3 +22,39 @@ pub fn get_ident_lower(component_value: &ComponentValue) -> Result<String, ()> {
         _ => Err(()),
     }
 }
+
+
+pub struct BufferedIter<E, I> {
+    iter: I,
+    buffer: Option<E>,
+}
+
+impl<E, I: Iterator<E>> BufferedIter<E, I> {
+    pub fn new(iter: I) -> BufferedIter<E, I> {
+        BufferedIter {
+            iter: iter,
+            buffer: None,
+        }
+    }
+
+    #[inline]
+    pub fn push_back(&mut self, value: E) {
+        assert!(self.buffer.is_none());
+        self.buffer = Some(value);
+    }
+}
+
+impl<E, I: Iterator<E>> Iterator<E> for BufferedIter<E, I> {
+    #[inline]
+    fn next(&mut self) -> Option<E> {
+        if self.buffer.is_some() {
+            self.buffer.take()
+        }
+        else {
+            self.iter.next()
+        }
+    }
+}
+
+
+pub type ParserIter<'a> = BufferedIter<&'a ComponentValue, SkipWhitespaceIterator<'a>>;
diff --git a/src/components/style/properties/mod.rs.mako b/src/components/style/properties/mod.rs.mako
index 60cf11270d8..c3677242ee6 100644
--- a/src/components/style/properties/mod.rs.mako
+++ b/src/components/style/properties/mod.rs.mako
@@ -718,61 +718,61 @@ pub mod longhands {
             pub type T = Vec<FontFamily>;
         }
         pub type SpecifiedValue = computed_value::T;
-        #[inline] pub fn get_initial_value() -> computed_value::T { vec!(FamilyName("serif".to_string())) }
+
+        #[inline]
+        pub fn get_initial_value() -> computed_value::T {
+            vec![FamilyName("serif".to_string())]
+        }
         /// <familiy-name>#
         /// <familiy-name> = <string> | [ <ident>+ ]
         /// TODO: <generic-familiy>
         pub fn parse(input: &[ComponentValue], _base_url: &Url) -> Result<SpecifiedValue, ()> {
             from_iter(input.skip_whitespace())
         }
-        pub fn from_iter<'a>(mut iter: SkipWhitespaceIterator<'a>) -> Result<SpecifiedValue, ()> {
-            let mut result = vec!();
-            macro_rules! add(
-                ($value: expr, $b: expr) => {
-                    {
-                        result.push($value);
-                        match iter.next() {
-                            Some(&Comma) => (),
-                            None => $b,
-                            _ => return Err(()),
-                        }
-                    }
+        pub fn from_iter<'a>(iter: SkipWhitespaceIterator<'a>) -> Result<SpecifiedValue, ()> {
+            let iter = &mut BufferedIter::new(iter);
+            let mut families = vec![try!(parse_one_family(iter))];
+            for component_value in iter {
+                match component_value {
+                    &Comma => {
+                        families.push(try!(parse_one_family(iter)));
+                    },
+                    _ => return Err(())
                 }
-            )
-            'outer: loop {
-                match iter.next() {
-                    // TODO: avoid copying strings?
-                    Some(&String(ref value)) => add!(FamilyName(value.clone()), break 'outer),
-                    Some(&Ident(ref value)) => {
-                        match value.as_slice().to_ascii_lower().as_slice() {
-//                            "serif" => add!(Serif, break 'outer),
-//                            "sans-serif" => add!(SansSerif, break 'outer),
-//                            "cursive" => add!(Cursive, break 'outer),
-//                            "fantasy" => add!(Fantasy, break 'outer),
-//                            "monospace" => add!(Monospace, break 'outer),
-                            _ => {
-                                let mut idents = vec!(value.as_slice());
-                                loop {
-                                    match iter.next() {
-                                        Some(&Ident(ref value)) => idents.push(value.as_slice()),
-                                        Some(&Comma) => {
-                                            result.push(FamilyName(idents.connect(" ")));
-                                            break
-                                        },
-                                        None => {
-                                            result.push(FamilyName(idents.connect(" ")));
-                                            break 'outer
-                                        },
-                                        _ => return Err(()),
-                                    }
-                                }
-                            }
-                        }
+            }
+            Ok(families)
+        }
+        pub fn parse_one_family<'a>(iter: &mut ParserIter) -> Result<FontFamily, ()> {
+            // TODO: avoid copying strings?
+            let mut idents = match iter.next() {
+                Some(&String(ref value)) => return Ok(FamilyName(value.clone())),
+                Some(&Ident(ref value)) => {
+//                    match value.as_slice().to_ascii_lower().as_slice() {
+//                        "serif" => return Ok(Serif),
+//                        "sans-serif" => return Ok(SansSerif),
+//                        "cursive" => return Ok(Cursive),
+//                        "fantasy" => return Ok(Fantasy),
+//                        "monospace" => return Ok(Monospace),
+//                        _ => {
+                            vec![value.as_slice()]
+//                        }
+//                    }
+                }
+                _ => return Err(())
+            };
+            for component_value in iter {
+                match component_value {
+                    &Ident(ref value) => {
+                        idents.push(value.as_slice());
+                        iter.next();
+                    },
+                    _ => {
+                        iter.push_back(component_value);
+                        break
                     }
-                    _ => return Err(()),
                 }
             }
-            Ok(result)
+            Ok(FamilyName(idents.connect(" ")))
         }
     </%self:longhand>