Bug 1412743: Avoid double-applying text-zoom for keywords. r?Manishearth draft
authorEmilio Cobos Álvarez <emilio@crisal.io>
Tue, 31 Oct 2017 12:07:33 +0100
changeset 689376 727af52a0f05e2a55513cb8f26bb2bdfeef0dd59
parent 689353 371e44e0034771ec8a5ac3c5a6518ef608227b99
child 689377 77400d2415d3018fd4f3121d992f0f2a8a1a054c
push id87004
push userbmo:emilio@crisal.io
push dateTue, 31 Oct 2017 13:41:52 +0000
reviewersManishearth
bugs1412743
milestone58.0a1
Bug 1412743: Avoid double-applying text-zoom for keywords. r?Manishearth MozReview-Commit-ID: 3iOstV6RifQ
servo/components/style/properties/longhand/font.mako.rs
servo/components/style/values/computed/font.rs
servo/components/style/values/specified/font.rs
--- a/servo/components/style/properties/longhand/font.mako.rs
+++ b/servo/components/style/properties/longhand/font.mako.rs
@@ -847,18 +847,17 @@ macro_rules! impl_gecko_keyword_conversi
         % if product == "gecko":
             // if the language or generic changed, we need to recalculate
             // the font size from the stored font-size origin information.
             if context.builder.get_font().gecko().mLanguage.mRawPtr !=
                context.builder.get_parent_font().gecko().mLanguage.mRawPtr ||
                context.builder.get_font().gecko().mGenericID !=
                context.builder.get_parent_font().gecko().mGenericID {
                 if let Some(info) = computed.keyword_info {
-                    computed.size = context.maybe_zoom_text(info.kw.to_computed_value(context)
-                                                                .scale_by(info.factor) + info.offset)
+                    computed.size = info.to_computed_value(context);
                 }
             }
         % endif
 
         let device = context.builder.device;
         let mut font = context.builder.take_font();
         let parent_unconstrained = {
             let parent_font = context.builder.get_parent_font();
@@ -880,18 +879,17 @@ macro_rules! impl_gecko_keyword_conversi
     /// 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.get_parent_font()
                                        .clone_font_size()
                                        .keyword_info.map(|info| {
-            context.maybe_zoom_text(SpecifiedValue::Keyword(info)
-                  .to_computed_value(context).size)
+            SpecifiedValue::Keyword(info).to_computed_value(context).size
         });
         let mut font = context.builder.take_font();
         font.inherit_font_size_from(context.builder.get_parent_font(),
                                     kw_inherited_size,
                                     context.builder.device);
         context.builder.put_font(font);
     }
 
@@ -899,19 +897,19 @@ macro_rules! impl_gecko_keyword_conversi
     ///
     /// 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?
     pub fn cascade_initial_font_size(context: &mut Context) {
         // font-size's default ("medium") does not always
         // compute to the same value and depends on the font
-        let mut computed = longhands::font_size::get_initial_specified_value()
-                                            .to_computed_value(context);
-        computed.size = context.maybe_zoom_text(computed.size);
+        let computed =
+            longhands::font_size::get_initial_specified_value()
+                .to_computed_value(context);
         context.builder.mutate_font().set_font_size(computed);
         % if product == "gecko":
             let device = context.builder.device;
             context.builder.mutate_font().fixup_font_min_size(device);
         % endif
     }
 </%helpers:longhand>
 
--- a/servo/components/style/values/computed/font.rs
+++ b/servo/components/style/values/computed/font.rs
@@ -33,16 +33,23 @@ pub struct KeywordInfo {
     pub kw: specified::KeywordSize,
     /// A factor to be multiplied by the computed size of the keyword
     pub factor: f32,
     /// An additional Au offset to add to the kw*factor in the case of calcs
     pub offset: NonNegativeLength,
 }
 
 impl KeywordInfo {
+    /// Computes the final size for this font-size keyword, accounting for
+    /// text-zoom.
+    pub fn to_computed_value(&self, context: &Context) -> NonNegativeLength {
+        let base = context.maybe_zoom_text(self.kw.to_computed_value(context));
+        base.scale_by(self.factor) + context.maybe_zoom_text(self.offset)
+    }
+
     /// Given a parent keyword info (self), apply an additional factor/offset to it
     pub fn compose(self, factor: f32, offset: NonNegativeLength) -> Self {
         KeywordInfo {
             kw: self.kw,
             factor: self.factor * factor,
             offset: self.offset.scale_by(factor) + offset,
         }
     }
--- a/servo/components/style/values/specified/font.rs
+++ b/servo/components/style/values/specified/font.rs
@@ -311,17 +311,17 @@ impl FontSize {
                     info = parent.keyword_info.map(|i| i.compose(ratio, abs));
                 }
                 let calc = calc.to_computed_value_zoomed(context, base_size);
                 calc.to_used_value(Some(base_size.resolve(context))).unwrap().into()
             }
             FontSize::Keyword(i) => {
                 // As a specified keyword, this is keyword derived
                 info = Some(i);
-                context.maybe_zoom_text(i.kw.to_computed_value(context).scale_by(i.factor) + i.offset)
+                i.to_computed_value(context)
             }
             FontSize::Smaller => {
                 info = compose_keyword(1. / LARGER_FONT_SIZE_RATIO);
                 FontRelativeLength::Em(1. / LARGER_FONT_SIZE_RATIO)
                     .to_computed_value(context, base_size).into()
             }
             FontSize::Larger => {
                 info = compose_keyword(LARGER_FONT_SIZE_RATIO);