Bug 1338936 - Part 5: stylo: Support font-family presentation attribute; r?emilio draft
authorManish Goregaokar <manishearth@gmail.com>
Sun, 12 Feb 2017 16:02:29 -0800
changeset 485639 10e4bd13cce80d4f9a367a88a5d455abe7d23b1e
parent 485638 e5175ea71658061e0b2f877d99ac50c56b1692c6
child 485640 d0d38c57588e34d3737526fb20e84cb38c81d488
push id45797
push userbmo:manishearth@gmail.com
push dateThu, 16 Feb 2017 23:47:53 +0000
reviewersemilio
bugs1338936
milestone54.0a1
Bug 1338936 - Part 5: stylo: Support font-family presentation attribute; r?emilio MozReview-Commit-ID: 6wg32flypt7
servo/components/style/properties/longhand/font.mako.rs
servo/components/style/properties/shorthand/font.mako.rs
servo/ports/geckolib/glue.rs
--- a/servo/components/style/properties/longhand/font.mako.rs
+++ b/servo/components/style/properties/longhand/font.mako.rs
@@ -14,17 +14,17 @@
     use values::HasViewportPercentage;
     use values::computed::ComputedValueAsSpecified;
     pub use self::computed_value::T as SpecifiedValue;
 
     impl ComputedValueAsSpecified for SpecifiedValue {}
     no_viewport_percentage!(SpecifiedValue);
 
     pub mod computed_value {
-        use cssparser::CssStringWriter;
+        use cssparser::{CssStringWriter, Parser};
         use std::fmt::{self, Write};
         use Atom;
         use style_traits::ToCss;
         pub use self::FontFamily as SingleComputedValue;
 
         #[derive(Debug, PartialEq, Eq, Clone, Hash)]
         #[cfg_attr(feature = "servo", derive(HeapSizeOf, Deserialize, Serialize))]
         pub enum FontFamily {
@@ -68,16 +68,63 @@
                     "sans-serif" => return FontFamily::Generic(atom!("sans-serif")),
                     "cursive" => return FontFamily::Generic(atom!("cursive")),
                     "fantasy" => return FontFamily::Generic(atom!("fantasy")),
                     "monospace" => return FontFamily::Generic(atom!("monospace")),
                     _ => {}
                 }
                 FontFamily::FamilyName(FamilyName(input))
             }
+
+            /// 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))))
+                }
+                let first_ident = try!(input.expect_ident());
+
+                // FIXME(bholley): The fast thing to do here would be to look up the
+                // string (as lowercase) in the static atoms table. We don't have an
+                // API to do that yet though, so we do the simple thing for now.
+                let mut css_wide_keyword = false;
+                match_ignore_ascii_case! { first_ident,
+                    "serif" => return Ok(FontFamily::Generic(atom!("serif"))),
+                    "sans-serif" => return Ok(FontFamily::Generic(atom!("sans-serif"))),
+                    "cursive" => return Ok(FontFamily::Generic(atom!("cursive"))),
+                    "fantasy" => return Ok(FontFamily::Generic(atom!("fantasy"))),
+                    "monospace" => return Ok(FontFamily::Generic(atom!("monospace"))),
+
+                    // https://drafts.csswg.org/css-fonts/#propdef-font-family
+                    // "Font family names that happen to be the same as a keyword value
+                    //  (‘inherit’, ‘serif’, ‘sans-serif’, ‘monospace’, ‘fantasy’, and ‘cursive’)
+                    //  must be quoted to prevent confusion with the keywords with the same names.
+                    //  The keywords ‘initial’ and ‘default’ are reserved for future use
+                    //  and must also be quoted when used as font names.
+                    //  UAs must not consider these keywords as matching the <family-name> type."
+                    "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.into_owned();
+                // 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_str(" ");
+                    value.push_str(&ident);
+                }
+                while let Ok(ident) = input.try(|input| input.expect_ident()) {
+                    value.push_str(" ");
+                    value.push_str(&ident);
+                }
+                Ok(FontFamily::FamilyName(FamilyName(Atom::from(value))))
+            }
         }
 
         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('"')
             }
@@ -114,85 +161,37 @@
     #[inline]
     pub fn get_initial_value() -> computed_value::T {
         computed_value::T(vec![FontFamily::Generic(atom!("serif"))])
     }
 
     /// <family-name>#
     /// <family-name> = <string> | [ <ident>+ ]
     /// TODO: <generic-family>
-    pub fn parse(context: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
-        Vec::<FontFamily>::parse(context, input).map(SpecifiedValue)
+    pub fn parse(_: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
+        SpecifiedValue::parse(input)
     }
 
-    impl Parse for Vec<FontFamily> {
-        fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-            input.parse_comma_separated(|input| FontFamily::parse(context, input))
+    impl SpecifiedValue {
+        pub fn parse(input: &mut Parser) -> Result<Self, ()> {
+            input.parse_comma_separated(|input| FontFamily::parse(input)).map(SpecifiedValue)
         }
     }
 
     /// `FamilyName::parse` is based on `FontFamily::parse` and not the other way around
     /// because we want the former to exclude generic family keywords.
     impl Parse for FamilyName {
-        fn parse(context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-            match FontFamily::parse(context, input) {
+        fn parse(_: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
+            match FontFamily::parse(input) {
                 Ok(FontFamily::FamilyName(name)) => Ok(name),
                 Ok(FontFamily::Generic(_)) |
                 Err(()) => Err(())
             }
         }
     }
-
-    impl Parse for FontFamily {
-        fn parse(_context: &ParserContext, input: &mut Parser) -> Result<Self, ()> {
-            if let Ok(value) = input.try(|input| input.expect_string()) {
-                return Ok(FontFamily::FamilyName(FamilyName(Atom::from(&*value))))
-            }
-            let first_ident = try!(input.expect_ident());
-
-            // FIXME(bholley): The fast thing to do here would be to look up the
-            // string (as lowercase) in the static atoms table. We don't have an
-            // API to do that yet though, so we do the simple thing for now.
-            let mut css_wide_keyword = false;
-            match_ignore_ascii_case! { first_ident,
-                "serif" => return Ok(FontFamily::Generic(atom!("serif"))),
-                "sans-serif" => return Ok(FontFamily::Generic(atom!("sans-serif"))),
-                "cursive" => return Ok(FontFamily::Generic(atom!("cursive"))),
-                "fantasy" => return Ok(FontFamily::Generic(atom!("fantasy"))),
-                "monospace" => return Ok(FontFamily::Generic(atom!("monospace"))),
-
-                // https://drafts.csswg.org/css-fonts/#propdef-font-family
-                // "Font family names that happen to be the same as a keyword value
-                //  (‘inherit’, ‘serif’, ‘sans-serif’, ‘monospace’, ‘fantasy’, and ‘cursive’)
-                //  must be quoted to prevent confusion with the keywords with the same names.
-                //  The keywords ‘initial’ and ‘default’ are reserved for future use
-                //  and must also be quoted when used as font names.
-                //  UAs must not consider these keywords as matching the <family-name> type."
-                "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.into_owned();
-            // 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_str(" ");
-                value.push_str(&ident);
-            }
-            while let Ok(ident) = input.try(|input| input.expect_ident()) {
-                value.push_str(" ");
-                value.push_str(&ident);
-            }
-            Ok(FontFamily::FamilyName(FamilyName(Atom::from(value))))
-        }
-    }
 </%helpers:longhand>
 
 
 ${helpers.single_keyword("font-style",
                          "normal italic oblique",
                          gecko_constant_prefix="NS_FONT_STYLE",
                          gecko_ffi_name="mFont.style",
                          spec="https://drafts.csswg.org/css-fonts/#propdef-font-style",
--- a/servo/components/style/properties/shorthand/font.mako.rs
+++ b/servo/components/style/properties/shorthand/font.mako.rs
@@ -7,20 +7,19 @@
 <%helpers:shorthand name="font" sub_properties="font-style font-variant font-weight font-stretch
                                                 font-size line-height font-family
                                                 ${'font-size-adjust' if product == 'gecko' else ''}
                                                 ${'font-kerning' if product == 'gecko' else ''}
                                                 ${'font-variant-caps' if product == 'gecko' else ''}
                                                 ${'font-variant-position' if product == 'gecko' else ''}
                                                 ${'font-language-override' if product == 'none' else ''}"
                     spec="https://drafts.csswg.org/css-fonts-3/#propdef-font">
-    use parser::Parse;
     use properties::longhands::{font_style, font_variant, font_weight, font_stretch};
-    use properties::longhands::{font_size, line_height, font_family};
-    use properties::longhands::font_family::computed_value::FontFamily;
+    use properties::longhands::{font_size, line_height};
+    use properties::longhands::font_family::SpecifiedValue as FontFamily;
 
     pub fn parse_value(context: &ParserContext, input: &mut Parser) -> Result<Longhands, ()> {
         let mut nb_normals = 0;
         let mut style = None;
         let mut variant = None;
         let mut weight = None;
         let mut stretch = None;
         let size;
@@ -66,25 +65,25 @@
         if size.is_none() || (count(&style) + count(&weight) + count(&variant) + count(&stretch) + nb_normals) > 4 {
             return Err(())
         }
         let line_height = if input.try(|input| input.expect_delim('/')).is_ok() {
             Some(try!(line_height::parse(context, input)))
         } else {
             None
         };
-        let family = Vec::<FontFamily>::parse(context, input)?;
+        let family = FontFamily::parse(input)?;
         Ok(Longhands {
             font_style: style,
             font_variant: variant,
             font_weight: weight,
             font_stretch: stretch,
             font_size: size,
             line_height: line_height,
-            font_family: Some(font_family::SpecifiedValue(family)),
+            font_family: Some(family),
     % if product == "gecko":
             font_size_adjust: None,
             font_kerning: None,
             font_variant_caps: None,
             font_variant_position: None,
     % endif
     % if product == "none":
             font_language_override: None,
--- a/servo/ports/geckolib/glue.rs
+++ b/servo/ports/geckolib/glue.rs
@@ -1165,20 +1165,32 @@ pub extern "C" fn Servo_DeclarationBlock
         BorderLeftColor => Box::new(color),
         Color => Box::new(CSSRGBA {parsed: rgba, authored: None}),
         BackgroundColor => Box::new(color),
     };
     declarations.write().declarations.push((prop, Default::default()));
 }
 
 #[no_mangle]
-pub extern "C" fn Servo_DeclarationBlock_SetFontFamily(_:
-                                                    RawServoDeclarationBlockBorrowed,
-                                                    _: *const nsAString) {
+pub extern "C" fn Servo_DeclarationBlock_SetFontFamily(declarations:
+                                                       RawServoDeclarationBlockBorrowed,
+                                                       value: *const nsAString) {
+    use cssparser::Parser;
+    use style::properties::{DeclaredValue, PropertyDeclaration};
+    use style::properties::longhands::font_family::SpecifiedValue as FontFamily;
 
+    let declarations = RwLock::<PropertyDeclarationBlock>::as_arc(&declarations);
+    let string = unsafe { (*value).to_string() };
+    let mut parser = Parser::new(&string);
+    if let Ok(family) = FontFamily::parse(&mut parser) {
+        if parser.is_exhausted() {
+            let decl = PropertyDeclaration::FontFamily(DeclaredValue::Value(family));
+            declarations.write().declarations.push((decl, Default::default()));
+        }
+    }
 }
 
 #[no_mangle]
 pub extern "C" fn Servo_DeclarationBlock_SetTextDecorationColorOverride(_:
                                                         RawServoDeclarationBlockBorrowed) {
 
 }