Bug 1391341 - stylo: inherit the computed value instead of the specified keyword for font-size property. draft
authorJeremy Chen <jeremychen@mozilla.com>
Mon, 28 Aug 2017 11:47:00 +0800
changeset 653840 00ad388fcb06c0cb871bf0d9b8adb01182c29097
parent 653766 d10c97627b51a226e19d0fa801201897fe1932f6
child 728431 fe960b2714bec746795eba3bd6433ddd41daf53c
push id76426
push userbmo:jeremychen@mozilla.com
push dateMon, 28 Aug 2017 04:13:00 +0000
bugs1391341
milestone57.0a1
Bug 1391341 - stylo: inherit the computed value instead of the specified keyword for font-size property. According to the spec [1], we should just inherit the computed value from the parent if the font-size property is unset or inherit. The current implementation in Stylo keeps tracking the specified keyword during the inheritance, and recomputes font-size in case of language changes. Althought this seems has a better interaction with language specific user prefs, this would conflict with the spec as currently written. To avoid the spec conflict and to match Gecko's behavior, let the font-size inheritance blindly inherit the computed value from the parent element. [1] https://www.w3.org/TR/css-fonts-3/#font-size-prop MozReview-Commit-ID: D4VXih4TKCk
servo/components/style/properties/gecko.mako.rs
servo/components/style/properties/longhand/font.mako.rs
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -2399,21 +2399,18 @@ fn static_assert() {
             (cmp::min(new_size, cmp::max(new_unconstrained_size, min)),
              new_unconstrained_size)
         }
     }
 
     /// This function will also handle scriptminsize and scriptlevel
     /// so should not be called when you just want the font sizes to be copied.
     /// Hence the different name.
-    ///
-    /// Returns true if the inherited keyword size was actually used
     pub fn inherit_font_size_from(&mut self, parent: &Self,
-                                  kw_inherited_size: Option<NonNegativeAu>,
-                                  device: &Device) -> bool {
+                                  device: &Device) {
         let (adjusted_size, adjusted_unconstrained_size)
             = self.calculate_script_level_size(parent, device);
         if adjusted_size.0 != parent.gecko.mSize ||
            adjusted_unconstrained_size.0 != parent.gecko.mScriptUnconstrainedSize {
             // This is incorrect. When there is both a keyword size being inherited
             // and a scriptlevel change, we must handle the keyword size the same
             // way we handle em units. This complicates things because we now have
             // to keep track of the adjusted and unadjusted ratios in the kw font size.
@@ -2425,31 +2422,21 @@ fn static_assert() {
             // values, and reusing those instead of font_size_keyword.
 
 
             // In the case that MathML has given us an adjusted size, apply it.
             // Keep track of the unconstrained adjusted size.
             self.gecko.mSize = adjusted_size.0;
             self.gecko.mScriptUnconstrainedSize = adjusted_unconstrained_size.0;
             self.fixup_font_min_size(device);
-            false
-        } else if let Some(size) = kw_inherited_size {
-            // Parent element was a keyword-derived size.
-            self.gecko.mSize = size.value();
-            // MathML constraints didn't apply here, so we can ignore this.
-            self.gecko.mScriptUnconstrainedSize = size.value();
-            self.fixup_font_min_size(device);
-            true
         } else {
-            // MathML isn't affecting us, and our parent element does not
-            // have a keyword-derived size. Set things normally.
+            // MathML isn't affecting us, so set things normally.
             self.gecko.mSize = parent.gecko.mSize;
             self.gecko.mScriptUnconstrainedSize = parent.gecko.mScriptUnconstrainedSize;
             self.fixup_font_min_size(device);
-            false
         }
     }
 
     pub fn clone_font_size(&self) -> longhands::font_size::computed_value::T {
         Au(self.gecko.mSize).into()
     }
 
     pub fn set_font_weight(&mut self, v: longhands::font_weight::computed_value::T) {
--- a/servo/components/style/properties/longhand/font.mako.rs
+++ b/servo/components/style/properties/longhand/font.mako.rs
@@ -1015,34 +1015,23 @@ macro_rules! impl_gecko_keyword_conversi
                    .mutate_font()
                    .apply_unconstrained_font_size(new_unconstrained);
         }
     }
 
     /// FIXME(emilio): This is very complex. Also, it should move to
     /// StyleBuilder.
     pub fn cascade_inherit_font_size(context: &mut Context) {
-        // If inheriting, we must recompute font-size in case of language
-        // changes using the font_size_keyword. We also need to do this to
-        // handle mathml scriptlevel changes
-        let kw_inherited_size = context.builder.font_size_keyword.map(|(kw, ratio)| {
-            context.maybe_zoom_text(SpecifiedValue::Keyword(kw, ratio).to_computed_value(context))
-        });
-        let parent_kw;
         let device = context.builder.device;
         let mut font = context.builder.take_font();
-        let used_kw = {
+        {
             let parent_font = context.builder.get_parent_font();
-            parent_kw = context.builder.inherited_font_computation_data().font_size_keyword;
-
-            font.inherit_font_size_from(parent_font, kw_inherited_size, device)
-        };
+            font.inherit_font_size_from(parent_font, device);
+        }
         context.builder.put_font(font);
-        context.builder.font_size_keyword =
-            if used_kw { parent_kw } else { None };
     }
 
     /// Cascade the initial value for the `font-size` property.
     ///
     /// FIXME(emilio): This is the only function that is outside of the
     /// `StyleBuilder`, and should really move inside!
     ///
     /// Can we move the font stuff there?