auto merge of #1455 : SimonSapin/servo/font-style, r=kmcallister

* Using .connect(", ") just to use .split(",") later is silly. Keep it a vector.
* The 'font-style' property can not be both italic and oblique. Use an enum instead of two booleans.

src/components/gfx/font.rs

@@ -15,7 +15,7 @@ use std::rc::RcMut;
 use servo_util::cache::{Cache, HashCache};
 use servo_util::range::Range;
 use servo_util::time::ProfilerChan;
-use style::computed_values::{text_decoration, font_weight};
+use style::computed_values::{text_decoration, font_weight, font_style};
 
 use color::Color;
 use font_context::FontContext;
@@ -100,9 +100,8 @@ pub struct FontMetrics {
 pub struct FontStyle {
     pt_size: f64,
     weight: font_weight::T,
-    italic: bool,
-    oblique: bool,
-    families: ~str,
+    style: font_style::T,
+    families: ~[~str],
     // TODO(Issue #198): font-stretch, text-decoration, font-variant, size-adjust
 }
 
@@ -150,7 +149,7 @@ pub enum FontSelector {
 // The ordering of font instances is mainly decided by the CSS
 // 'font-family' property. The last font is a system fallback font.
 pub struct FontGroup {
-    families: ~str,
+    families: ~[~str],
     // style of the first western font in group, which is
     // used for purposes of calculating text run metrics.
     style: UsedFontStyle,
@@ -158,7 +157,7 @@ pub struct FontGroup {
 }
 
 impl FontGroup {
-    pub fn new(families: ~str, style: &UsedFontStyle, fonts: ~[RcMut<Font>]) -> FontGroup {
+    pub fn new(families: ~[~str], style: &UsedFontStyle, fonts: ~[RcMut<Font>]) -> FontGroup {
         FontGroup {
             families: families,
             style: (*style).clone(),
@@ -199,7 +198,7 @@ impl RunMetrics {
         // ascent+descent and advance is sometimes too generous and
         // looking at actual glyph extents can yield a tighter box.
 
-        RunMetrics { 
+        RunMetrics {
             advance_width: advance,
             bounding_box: bounds,
             ascent: ascent,
@@ -288,9 +287,9 @@ impl<'self> Font {
     fn make_shaper(&'self mut self) -> &'self Shaper {
         // fast path: already created a shaper
         match self.shaper {
-            Some(ref shaper) => { 
+            Some(ref shaper) => {
                 let s: &'self Shaper = shaper;
-                return s; 
+                return s;
             },
             None => {}
         }
@@ -401,7 +400,7 @@ impl Font {
 
         let glyphbuf = struct__AzGlyphBuffer {
             mGlyphs: vec::raw::to_ptr(azglyphs),
-            mNumGlyphs: azglyph_buf_len as uint32_t            
+            mNumGlyphs: azglyph_buf_len as uint32_t
         };
 
         unsafe {

src/components/gfx/font_context.rs

@@ -38,7 +38,7 @@ impl<'self> FontContext {
            profiler_chan: ProfilerChan)
            -> FontContext {
         let handle = FontContextHandle::new();
-        let font_list = if needs_font_list { 
+        let font_list = if needs_font_list {
                             Some(FontList::new(&handle, profiler_chan.clone())) }
                         else { None };
 
@@ -50,7 +50,7 @@ impl<'self> FontContext {
         generic_fonts.insert(~"fantasy", ~"Papyrus");
         generic_fonts.insert(~"monospace", ~"Menlo");
 
-        FontContext { 
+        FontContext {
             instance_cache: LRUCache::new(10),
             font_list: font_list,
             group_cache: LRUCache::new(10),
@@ -86,7 +86,7 @@ impl<'self> FontContext {
                 debug!("font cache hit");
                 Ok(f)
             },
-            None => { 
+            None => {
                 debug!("font cache miss");
                 let result = self.create_font_instance(desc);
                 match result.clone() {
@@ -99,12 +99,10 @@ impl<'self> FontContext {
         }
     }
 
-    fn transform_family(&self, family: &str) -> ~str {
-        // FIXME: Need a find_like() in HashMap.
-        let family = family.to_str();
-        debug!("(transform family) searching for `{:s}`", family);
-        match self.generic_fonts.find(&family) {
-            None => family,
+    fn transform_family(&self, family: &~str) -> ~str {
+        debug!("(transform family) searching for `{:s}`", family.as_slice());
+        match self.generic_fonts.find(family) {
+            None => family.to_owned(),
             Some(mapped_family) => (*mapped_family).clone()
         }
     }
@@ -115,9 +113,8 @@ impl<'self> FontContext {
         debug!("(create font group) --- starting ---");
 
         // TODO(Issue #193): make iteration over 'font-family' more robust.
-        for family in style.families.split_iter(',') {
-            let family_name = family.trim();
-            let transformed_family_name = self.transform_family(family_name);
+        for family in style.families.iter() {
+            let transformed_family_name = self.transform_family(family);
             debug!("(create font group) transformed family is `{:s}`", transformed_family_name);
             let mut found = false;
 
@@ -145,7 +142,7 @@ impl<'self> FontContext {
                     let instance = self.get_font_by_descriptor(result);
 
                     for font in instance.iter() { fonts.push(font.clone()); }
-                }, 
+                },
                 _ => {}
             }
 
@@ -197,13 +194,13 @@ impl<'self> FontContext {
 
         debug!("(create font group) --- finished ---");
 
-        unsafe { RcMut::new_unchecked(FontGroup::new(style.families.to_owned(), &used_style, fonts)) }
+        unsafe { RcMut::new_unchecked(FontGroup::new(style.families.clone(), &used_style, fonts)) }
     }
 
     fn create_font_instance(&self, desc: &FontDescriptor) -> Result<RcMut<Font>, ()> {
         return match &desc.selector {
             // TODO(Issue #174): implement by-platform-name font selectors.
-            &SelectorPlatformIdentifier(ref identifier) => { 
+            &SelectorPlatformIdentifier(ref identifier) => {
                 let result_handle = self.handle.create_font_from_identifier((*identifier).clone(),
                                                                             desc.style.clone());
                 do result_handle.and_then |handle| {

src/components/gfx/font_list.rs

@@ -10,7 +10,7 @@ use platform::font_list::FontListHandle;
 use servo_util::time;
 use servo_util::time::profile;
 use servo_util::time::ProfilerChan;
-use style::computed_values::font_weight;
+use style::computed_values::{font_weight, font_style};
 
 use std::hashmap::HashMap;
 
@@ -54,7 +54,7 @@ impl<'self> FontList {
     }
 
     pub fn find_font_in_family(&'self mut self,
-                           family_name: &~str, 
+                           family_name: &~str,
                            style: &SpecifiedFontStyle) -> Option<&'self FontEntry> {
         // TODO(Issue #188): look up localized font family names if canonical name not found
         // look up canonical name
@@ -82,7 +82,7 @@ impl<'self> FontList {
     }
 }
 
-// Holds a specific font family, and the various 
+// Holds a specific font family, and the various
 pub struct FontFamily<'self> {
     family_name: ~str,
     entries: ~[FontEntry],
@@ -115,8 +115,8 @@ impl<'self> FontFamily {
         // TODO(Issue #190): if not in the fast path above, do
         // expensive matching of weights, etc.
         for entry in self.entries.iter() {
-            if (style.weight.is_bold() == entry.is_bold()) && 
-               (style.italic == entry.is_italic()) {
+            if (style.weight.is_bold() == entry.is_bold()) &&
+               ((style.style == font_style::italic) == entry.is_italic()) {
 
                 return Some(entry);
             }

src/components/gfx/platform/android/font_list.rs

@@ -17,6 +17,8 @@ use fontconfig::fontconfig::{
     FcObjectSetAdd, FcPatternGetInteger
 };
 
+use style::computed_values::font_style;
+
 
 use font::{FontHandleMethods, UsedFontStyle};
 use font_list::{FontEntry, FontFamily, FontFamilyMap};
@@ -163,23 +165,25 @@ pub fn path_from_identifier(name: ~str, style: &UsedFontStyle) -> Result<~str, (
             return Err(());
         }
 
-        if style.italic {
-            let res = do "slant".to_c_str().with_ref |FC_SLANT| {
-                FcPatternAddInteger(pattern, FC_SLANT, FC_SLANT_ITALIC)
-            };
-            if res != 1 {
-                debug!("adding slant(italic) to pattern failed");
-                return Err(());
-            }
-        }
-
-        if style.oblique {
-            let res = do "slant".to_c_str().with_ref |FC_SLANT| {
-                FcPatternAddInteger(pattern, FC_SLANT, FC_SLANT_OBLIQUE)
-            };
-            if res != 1 {
-                debug!("adding slant(oblique) to pattern failed");
-                return Err(());
+        match style.style {
+            font_style::normal => (),
+            font_style::italic => {
+                let res = do "slant".to_c_str().with_ref |FC_SLANT| {
+                    FcPatternAddInteger(pattern, FC_SLANT, FC_SLANT_ITALIC)
+                };
+                if res != 1 {
+                    debug!("adding slant to pattern failed");
+                    return Err(());
+                }
+            },
+            font_style::oblique => {
+                let res = do "slant".to_c_str().with_ref |FC_SLANT| {
+                    FcPatternAddInteger(pattern, FC_SLANT, FC_SLANT_OBLIQUE)
+                };
+                if res != 1 {
+                    debug!("adding slant(oblique) to pattern failed");
+                    return Err(());
+                }
             }
         }
 

src/components/gfx/platform/linux/font_list.rs

@@ -17,6 +17,8 @@ use fontconfig::fontconfig::{
     FcObjectSetAdd, FcPatternGetInteger
 };
 
+use style::computed_values::font_style;
+
 
 use font::{FontHandleMethods, UsedFontStyle};
 use font_list::{FontEntry, FontFamily, FontFamilyMap};
@@ -163,23 +165,25 @@ pub fn path_from_identifier(name: ~str, style: &UsedFontStyle) -> Result<~str, (
             return Err(());
         }
 
-        if style.italic {
-            let res = do "slant".to_c_str().with_ref |FC_SLANT| {
-                FcPatternAddInteger(pattern, FC_SLANT, FC_SLANT_ITALIC)
-            };
-            if res != 1 {
-                debug!("adding slant(italic) to pattern failed");
-                return Err(());
-            }
-        }
-
-        if style.oblique {
-            let res = do "slant".to_c_str().with_ref |FC_SLANT| {
-                FcPatternAddInteger(pattern, FC_SLANT, FC_SLANT_OBLIQUE)
-            };
-            if res != 1 {
-                debug!("adding slant(oblique) to pattern failed");
-                return Err(());
+        match style.style {
+            font_style::normal => (),
+            font_style::italic => {
+                let res = do "slant".to_c_str().with_ref |FC_SLANT| {
+                    FcPatternAddInteger(pattern, FC_SLANT, FC_SLANT_ITALIC)
+                };
+                if res != 1 {
+                    debug!("adding slant to pattern failed");
+                    return Err(());
+                }
+            },
+            font_style::oblique => {
+                let res = do "slant".to_c_str().with_ref |FC_SLANT| {
+                    FcPatternAddInteger(pattern, FC_SLANT, FC_SLANT_OBLIQUE)
+                };
+                if res != 1 {
+                    debug!("adding slant(oblique) to pattern failed");
+                    return Err(());
+                }
             }
         }
 

src/components/main/layout/box.rs

@@ -29,7 +29,7 @@ use std::cmp::ApproxEq;
 use std::num::Zero;
 use style::{ComputedValues, TElement, TNode};
 use style::computed_values::{LengthOrPercentage, overflow};
-use style::computed_values::{border_style, clear, font_family, font_style, line_height};
+use style::computed_values::{border_style, clear, font_family, line_height};
 use style::computed_values::{text_align, text_decoration, vertical_align, visibility};
 
 use css::node_style::StyledNode;
@@ -404,25 +404,15 @@ impl Box {
                 font_family::FamilyName(ref name) => (*name).clone(),
             }
         };
-        let font_families = font_families.connect(", ");
-        debug!("(font style) font families: `{:s}`", font_families);
+        debug!("(font style) font families: `{:?}`", font_families);
 
         let font_size = my_style.Font.font_size.to_f64().unwrap() / 60.0;
         debug!("(font style) font size: `{:f}px`", font_size);
 
-        let (italic, oblique) = match my_style.Font.font_style {
-            font_style::normal => (false, false),
-            font_style::italic => (true, false),
-            font_style::oblique => (false, true),
-        };
-
-        let font_weight = my_style.Font.font_weight;
-
         FontStyle {
             pt_size: font_size,
-            weight: font_weight,
-            italic: italic,
-            oblique: oblique,
+            weight: my_style.Font.font_weight,
+            style: my_style.Font.font_style,
             families: font_families,
         }
     }

src/components/style/properties.rs.mako

@@ -191,7 +191,7 @@ pub mod longhands {
         match component_value {
             &Ident(ref value) => {
                 // FIXME: Workaround for https://github.com/mozilla/rust/issues/10683
-                let value_lower = value.to_ascii_lower(); 
+                let value_lower = value.to_ascii_lower();
                 match value_lower.as_slice() {
                     "thin" => Some(specified::Length::from_px(1.)),
                     "medium" => Some(specified::Length::from_px(3.)),
@@ -236,7 +236,7 @@ pub mod longhands {
 
     // TODO: don't parse values we don't support
     <%self:single_keyword_computed name="display"
-            values="inline block inline-block 
+            values="inline block inline-block
             table inline-table table-row-group table-header-group table-footer-group
             table-row table-column-group table-column table-cell table-caption
             list-item
@@ -346,7 +346,7 @@ pub mod longhands {
             match input {
                 &Ident(ref value) => {
                     // FIXME: Workaround for https://github.com/mozilla/rust/issues/10683
-                    let value_lower = value.to_ascii_lower(); 
+                    let value_lower = value.to_ascii_lower();
                     match value_lower.as_slice() {
                         % for keyword in vertical_align_keywords:
                         "${keyword}" => Some(Specified_${to_rust_ident(keyword)}),
@@ -472,7 +472,7 @@ pub mod longhands {
                     Some(&Ident(ref value)) => {
                         // FIXME: Workaround for https://github.com/mozilla/rust/issues/10683
                         let value = value.as_slice();
-                        let value_lower = value.to_ascii_lower(); 
+                        let value_lower = value.to_ascii_lower();
                         match value_lower.as_slice() {
 //                            "serif" => add!(Serif),
 //                            "sans-serif" => add!(SansSerif),
@@ -523,7 +523,7 @@ pub mod longhands {
             match input {
                 &Ident(ref value) => {
                     // FIXME: Workaround for https://github.com/mozilla/rust/issues/10683
-                    let value_lower = value.to_ascii_lower();  
+                    let value_lower = value.to_ascii_lower();
                     match value_lower.as_slice() {
                         "bold" => Some(SpecifiedWeight700),
                         "normal" => Some(SpecifiedWeight400),