Bug 1355707 - stylo: Cascade relative font-sizes applied to keyword sizes; r?heycam draft
authorManish Goregaokar <manishearth@gmail.com>
Wed, 12 Apr 2017 14:23:33 +0800
changeset 561103 a5ceec6a0881765f4786c877b267376d84abe470
parent 560872 b05ee1233c7249972c289f4fea3ae6b95c53c927
child 562435 2679538e4b4f6ae3d285b8664257a594068761fe
push id53636
push userbmo:manishearth@gmail.com
push dateWed, 12 Apr 2017 08:16:32 +0000
reviewersheycam
bugs1355707
milestone55.0a1
Bug 1355707 - stylo: Cascade relative font-sizes applied to keyword sizes; r?heycam MozReview-Commit-ID: JGDzdGJiEGz
layout/reftests/bidi/reftest-stylo.list
layout/reftests/font-matching/reftest-stylo.list
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/helpers.mako.rs
servo/components/style/properties/longhand/font.mako.rs
servo/components/style/properties/properties.mako.rs
--- a/layout/reftests/bidi/reftest-stylo.list
+++ b/layout/reftests/bidi/reftest-stylo.list
@@ -43,19 +43,19 @@ fails == unicode-bidi-isolate-aharon.htm
 fails == unicode-bidi-plaintext-textarea-1.html unicode-bidi-plaintext-textarea-1.html
 fails == unicode-bidi-plaintext-textarea-2.html unicode-bidi-plaintext-textarea-2.html
 fails == unicode-bidi-plaintext-textarea-3.html unicode-bidi-plaintext-textarea-3.html
 fails == unicode-bidi-plaintext-textarea-4.html unicode-bidi-plaintext-textarea-4.html
 == with-first-letter-1a.html with-first-letter-1a.html
 == with-first-letter-1b.html with-first-letter-1b.html
 == with-first-letter-2a.html with-first-letter-2a.html
 == with-first-letter-2b.html with-first-letter-2b.html
-fails == 83958-1a.html 83958-1a.html
-fails == 83958-1b.html 83958-1b.html
-fails == 83958-1c.html 83958-1c.html
+== 83958-1a.html 83958-1a.html
+== 83958-1b.html 83958-1b.html
+== 83958-1c.html 83958-1c.html
 == 83958-2a.html 83958-2a.html
 == 83958-2b.html 83958-2b.html
 == 115921-1.html 115921-1.html
 fails == 115921-2.html 115921-2.html
 == 151407-1.html 151407-1.html
 == 151407-1a.html 151407-1a.html
 == 151407-2-ltr.html 151407-2-ltr.html
 == 151407-2-rtl.html 151407-2-rtl.html
--- a/layout/reftests/font-matching/reftest-stylo.list
+++ b/layout/reftests/font-matching/reftest-stylo.list
@@ -96,17 +96,17 @@ random-if(!(cocoaWidget||winWidget)) == 
 HTTP(..) == font-synthesis-1.html font-synthesis-1.html
 HTTP(..) == font-synthesis-2.html font-synthesis-2.html
 
 # Bug 1060791 - support for format 10 cmap in Apple Symbols;
 # relevant fonts not present on other platforms.
 == apple-symbols-1.html apple-symbols-1.html
 
 # distinguish between italic and oblique
-fails == simple-oblique.html simple-oblique.html
+== simple-oblique.html simple-oblique.html
 == italic-oblique-1.html italic-oblique-1.html
 == italic-oblique-2.html italic-oblique-2.html
 == italic-oblique-3.html italic-oblique-3.html
 == italic-oblique-4.html italic-oblique-4.html
 == italic-oblique-5.html italic-oblique-5.html
 == italic-oblique-6.html italic-oblique-6.html
 == italic-oblique-7.html italic-oblique-7.html
 == italic-oblique-8.html italic-oblique-8.html
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -74,17 +74,31 @@ pub mod style_structs {
 pub struct ComputedValues {
     % for style_struct in data.style_structs:
     ${style_struct.ident}: Arc<style_structs::${style_struct.name}>,
     % endfor
 
     custom_properties: Option<Arc<ComputedValuesMap>>,
     pub writing_mode: WritingMode,
     pub root_font_size: Au,
-    pub font_size_keyword: Option<longhands::font_size::KeywordSize>,
+    /// font-size keyword values (and font-size-relative values applied
+    /// to keyword values) need to preserve their identity as originating
+    /// from keywords and relative font sizes. We store this information
+    /// out of band in the ComputedValues. When None, the font size on the
+    /// current struct was computed from a value that was not a keyword
+    /// or a chain of font-size-relative values applying to successive parents
+    /// terminated by a keyword. When Some, this means the font-size was derived
+    /// from a keyword value or a keyword value on some ancestor with only
+    /// font-size-relative keywords and regular inheritance in between. The
+    /// integer stores the final ratio of the chain of font size relative values.
+    /// and is 1 when there was just a keyword and no relative values.
+    ///
+    /// When this is Some, we compute font sizes by computing the keyword against
+    /// the generic font, and then multiplying it by the ratio.
+    pub font_size_keyword: Option<(longhands::font_size::KeywordSize, f32)>,
 }
 
 impl ComputedValues {
     pub fn inherit_from(parent: &Self, default: &Self) -> Arc<Self> {
         Arc::new(ComputedValues {
             custom_properties: parent.custom_properties.clone(),
             writing_mode: parent.writing_mode,
             root_font_size: parent.root_font_size,
@@ -97,17 +111,17 @@ impl ComputedValues {
             % endif
             % endfor
         })
     }
 
     pub fn new(custom_properties: Option<Arc<ComputedValuesMap>>,
            writing_mode: WritingMode,
            root_font_size: Au,
-           font_size_keyword: Option<longhands::font_size::KeywordSize>,
+           font_size_keyword: Option<(longhands::font_size::KeywordSize, f32)>,
             % for style_struct in data.style_structs:
            ${style_struct.ident}: Arc<style_structs::${style_struct.name}>,
             % endfor
     ) -> Self {
         ComputedValues {
             custom_properties: custom_properties,
             writing_mode: writing_mode,
             root_font_size: root_font_size,
@@ -118,17 +132,17 @@ impl ComputedValues {
         }
     }
 
     pub fn default_values(pres_context: RawGeckoPresContextBorrowed) -> Arc<Self> {
         Arc::new(ComputedValues {
             custom_properties: None,
             writing_mode: WritingMode::empty(), // FIXME(bz): This seems dubious
             root_font_size: longhands::font_size::get_initial_value(), // FIXME(bz): Also seems dubious?
-            font_size_keyword: Some(Default::default()),
+            font_size_keyword: Some((Default::default(), 1.)),
             % for style_struct in data.style_structs:
                 ${style_struct.ident}: style_structs::${style_struct.name}::default(pres_context),
             % endfor
         })
     }
 
     #[inline]
     pub fn is_display_contents(&self) -> bool {
--- a/servo/components/style/properties/helpers.mako.rs
+++ b/servo/components/style/properties/helpers.mako.rs
@@ -262,18 +262,30 @@
                         % if property.logical:
                             let wm = context.style.writing_mode;
                         % endif
                         <% maybe_wm = ", wm" if property.logical else "" %>
                         match *value {
                             DeclaredValue::Value(ref specified_value) => {
                                 let computed = specified_value.to_computed_value(context);
                                 % if property.ident == "font_size":
-                                    if let longhands::font_size::SpecifiedValue::Keyword(kw) = **specified_value {
-                                        context.mutate_style().font_size_keyword = Some(kw);
+                                    if let longhands::font_size::SpecifiedValue::Keyword(kw, fraction) = **specified_value {
+                                        context.mutate_style().font_size_keyword = Some((kw, fraction));
+                                    } else if let Some(ratio) = specified_value.as_font_ratio() {
+                                        // In case a font-size-relative value was applied to a keyword
+                                        // value, we must preserve this fact in case the generic font family
+                                        // changes. relative values (em and %) applied to keywords must be
+                                        // recomputed from the base size for the keyword and the relative size.
+                                        //
+                                        // See bug 1355707
+                                        if let Some((kw, fraction)) = context.inherited_style().font_size_keyword {
+                                            context.mutate_style().font_size_keyword = Some((kw, fraction * ratio));
+                                        } else {
+                                            context.mutate_style().font_size_keyword = None;
+                                        }
                                     } else {
                                         context.mutate_style().font_size_keyword = None;
                                     }
                                 % endif
                                 % if property.has_uncacheable_values:
                                 context.mutate_style().mutate_${data.current_style_struct.name_lower}()
                                                       .set_${property.ident}(computed, cacheable ${maybe_wm});
                                 % else:
@@ -289,17 +301,17 @@
                                 CSSWideKeyword::Initial => {
                                     % if property.ident == "font_size":
                                         // font-size's default ("medium") does not always
                                         // compute to the same value and depends on the font
                                         let computed = longhands::font_size::get_initial_specified_value()
                                                             .to_computed_value(context);
                                         context.mutate_style().mutate_${data.current_style_struct.name_lower}()
                                                .set_font_size(computed);
-                                        context.mutate_style().font_size_keyword = Some(Default::default());
+                                        context.mutate_style().font_size_keyword = Some((Default::default(), 1.));
                                     % else:
                                         // We assume that it's faster to use copy_*_from rather than
                                         // set_*(get_initial_value());
                                         let initial_struct = default_style
                                                             .get_${data.current_style_struct.name_lower}();
                                         context.mutate_style().mutate_${data.current_style_struct.name_lower}()
                                                             .copy_${property.ident}_from(initial_struct ${maybe_wm});
                                     % endif
--- a/servo/components/style/properties/longhand/font.mako.rs
+++ b/servo/components/style/properties/longhand/font.mako.rs
@@ -410,23 +410,24 @@
 </%helpers:longhand>
 
 <%helpers:longhand name="font-size" need_clone="True" animation_type="normal"
                    spec="https://drafts.csswg.org/css-fonts/#propdef-font-size">
     use app_units::Au;
     use std::fmt;
     use style_traits::ToCss;
     use values::{FONT_MEDIUM_PX, HasViewportPercentage};
-    use values::specified::{LengthOrPercentage, Length, NoCalcLength, Percentage};
+    use values::specified::{FontRelativeLength, LengthOrPercentage, Length};
+    use values::specified::{NoCalcLength, Percentage};
 
     impl ToCss for SpecifiedValue {
         fn to_css<W>(&self, dest: &mut W) -> fmt::Result where W: fmt::Write {
             match *self {
                 SpecifiedValue::Length(ref lop) => lop.to_css(dest),
-                SpecifiedValue::Keyword(kw) => kw.to_css(dest),
+                SpecifiedValue::Keyword(kw, _) => kw.to_css(dest),
                 SpecifiedValue::Smaller => dest.write_str("smaller"),
                 SpecifiedValue::Larger => dest.write_str("larger"),
             }
         }
     }
 
     impl HasViewportPercentage for SpecifiedValue {
         fn has_viewport_percentage(&self) -> bool {
@@ -436,17 +437,23 @@
             }
         }
     }
 
     #[derive(Debug, Clone, PartialEq)]
     #[cfg_attr(feature = "servo", derive(HeapSizeOf))]
     pub enum SpecifiedValue {
         Length(specified::LengthOrPercentage),
-        Keyword(KeywordSize),
+        /// A keyword value, along with a ratio.
+        /// The ratio in any specified keyword value
+        /// will be 1, but we cascade keywordness even
+        /// after font-relative (percent and em) values
+        /// have been applied, which is where the keyword
+        /// comes in. See bug 1355707
+        Keyword(KeywordSize, f32),
         Smaller,
         Larger,
     }
 
     pub mod computed_value {
         use app_units::Au;
         pub type T = Au;
     }
@@ -591,29 +598,44 @@
                 0 | 1 => XSmall,
                 2 => Small,
                 3 => Medium,
                 4 => Large,
                 5 => XLarge,
                 6 => XXLarge,
                 // If value is greater than 7, let it be 7.
                 _ => XXXLarge,
-            })
+            }, 1.)
+        }
+
+        /// If this value is specified as a ratio of the parent font (em units or percent)
+        /// return the ratio
+        pub fn as_font_ratio(&self) -> Option<f32> {
+            if let SpecifiedValue::Length(ref lop) = *self {
+                if let LengthOrPercentage::Percentage(pc) = *lop {
+                    return Some(pc.0)
+                } else if let LengthOrPercentage::Length(ref nocalc) = *lop {
+                    if let NoCalcLength::FontRelative(FontRelativeLength::Em(em)) = *nocalc {
+                        return Some(em)
+                    }
+                }
+            }
+            None
         }
     }
 
     #[inline]
     #[allow(missing_docs)]
     pub fn get_initial_value() -> computed_value::T {
         Au::from_px(FONT_MEDIUM_PX)
     }
 
     #[inline]
     pub fn get_initial_specified_value() -> SpecifiedValue {
-        SpecifiedValue::Keyword(Medium)
+        SpecifiedValue::Keyword(Medium, 1.)
     }
 
     impl ToComputedValue for SpecifiedValue {
         type ComputedValue = computed_value::T;
 
         #[inline]
         fn to_computed_value(&self, context: &Context) -> computed_value::T {
             use values::specified::length::FontRelativeLength;
@@ -632,18 +654,18 @@
                 SpecifiedValue::Length(LengthOrPercentage::Percentage(Percentage(value))) => {
                     context.inherited_style().get_font().clone_font_size().scale_by(value)
                 }
                 SpecifiedValue::Length(LengthOrPercentage::Calc(ref calc)) => {
                     let calc = calc.to_computed_value(context);
                     calc.length() + context.inherited_style().get_font().clone_font_size()
                                            .scale_by(calc.percentage())
                 }
-                SpecifiedValue::Keyword(ref key) => {
-                    key.to_computed_value(context)
+                SpecifiedValue::Keyword(ref key, fraction) => {
+                    key.to_computed_value(context).scale_by(fraction)
                 }
                 SpecifiedValue::Smaller => {
                     FontRelativeLength::Em(0.85).to_computed_value(context,
                                                                    /* use_inherited */ true)
                 }
                 SpecifiedValue::Larger => {
                     FontRelativeLength::Em(1.2).to_computed_value(context,
                                                                    /* use_inherited */ true)
@@ -660,17 +682,17 @@
     }
     /// <length> | <percentage> | <absolute-size> | <relative-size>
     pub fn parse(_: &ParserContext, input: &mut Parser) -> Result<SpecifiedValue, ()> {
         if let Ok(lop) = input.try(specified::LengthOrPercentage::parse_non_negative) {
             return Ok(SpecifiedValue::Length(lop))
         }
 
         if let Ok(kw) = input.try(KeywordSize::parse) {
-            return Ok(SpecifiedValue::Keyword(kw))
+            return Ok(SpecifiedValue::Keyword(kw, 1.))
         }
 
         match_ignore_ascii_case! {&*input.expect_ident()?,
             "smaller" => Ok(SpecifiedValue::Smaller),
             "larger" => Ok(SpecifiedValue::Larger),
             _ => Err(())
         }
     }
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -1558,26 +1558,26 @@ pub struct ComputedValues {
         ${style_struct.ident}: Arc<style_structs::${style_struct.name}>,
     % endfor
     custom_properties: Option<Arc<::custom_properties::ComputedValuesMap>>,
     /// The writing mode of this computed values struct.
     pub writing_mode: WritingMode,
     /// The root element's computed font size.
     pub root_font_size: Au,
     /// The keyword behind the current font-size property, if any
-    pub font_size_keyword: Option<longhands::font_size::KeywordSize>,
+    pub font_size_keyword: Option<(longhands::font_size::KeywordSize, f32)>,
 }
 
 #[cfg(feature = "servo")]
 impl ComputedValues {
     /// Construct a `ComputedValues` instance.
     pub fn new(custom_properties: Option<Arc<::custom_properties::ComputedValuesMap>>,
                writing_mode: WritingMode,
                root_font_size: Au,
-               font_size_keyword: Option<longhands::font_size::KeywordSize>,
+               font_size_keyword: Option<(longhands::font_size::KeywordSize, f32)>,
             % for style_struct in data.active_style_structs():
                ${style_struct.ident}: Arc<style_structs::${style_struct.name}>,
             % endfor
     ) -> Self {
         ComputedValues {
             custom_properties: custom_properties,
             writing_mode: writing_mode,
             root_font_size: root_font_size,
@@ -1899,17 +1899,17 @@ mod lazy_static_module {
                     % if style_struct.name == "Font":
                         hash: 0,
                     % endif
                 }),
             % endfor
             custom_properties: None,
             writing_mode: WritingMode::empty(),
             root_font_size: longhands::font_size::get_initial_value(),
-            font_size_keyword: Some(Default::default()),
+            font_size_keyword: Some((Default::default(), 1)),
         };
     }
 }
 
 /// A per-longhand function that performs the CSS cascade for that longhand.
 pub type CascadePropertyFn =
     extern "Rust" fn(declaration: &PropertyDeclaration,
                      inherited_style: &ComputedValues,
@@ -2194,22 +2194,22 @@ pub fn apply_declarations<'a, F, I>(devi
                 let discriminant = LonghandId::FontSize as usize;
                 (CASCADE_PROPERTY[discriminant])(declaration,
                                                  inherited_style,
                                                  default_style,
                                                  &mut context,
                                                  &mut cacheable,
                                                  &mut cascade_info,
                                                  error_reporter);
-            } else if let Some(kw) = inherited_style.font_size_keyword {
+            } else if let Some((kw, fraction)) = inherited_style.font_size_keyword {
                 // Font size keywords will inherit as keywords and be recomputed
                 // each time.
                 let discriminant = LonghandId::FontSize as usize;
                 let size = PropertyDeclaration::FontSize(
-                    longhands::font_size::SpecifiedValue::Keyword(kw)
+                    longhands::font_size::SpecifiedValue::Keyword(kw, fraction)
                 );
                 (CASCADE_PROPERTY[discriminant])(&size,
                                                  inherited_style,
                                                  default_style,
                                                  &mut context,
                                                  &mut cacheable,
                                                  &mut cascade_info,
                                                  error_reporter);