Auto merge of #19807 - emilio:calc-font-size, r=Manishearth

style: Make the Gecko font-size calc() code do what it means to do.

It makes no sense to pass a custom base size of zero in presence of rem, ex, or
ch units.

Bug: 1431031
Reviewed-by: Manishearth
MozReview-Commit-ID: 7ZZwRzQKREX

<!-- Reviewable:start -->
---
This change is [<img src="https://reviewable.io/review_button.svg" height="34" align="absmiddle" alt="Reviewable"/>](https://reviewable.io/reviews/servo/servo/19807)
<!-- Reviewable:end -->

components/style/properties/gecko.mako.rs

@@ -2505,9 +2505,12 @@ fn static_assert() {
 
     /// Set font size, taking into account scriptminsize and scriptlevel
     /// Returns Some(size) if we have to recompute the script unconstrained size
-    pub fn apply_font_size(&mut self, v: FontSize,
+    pub fn apply_font_size(
-                           parent: &Self,
+        &mut self,
-                           device: &Device) -> Option<NonNegativeLength> {
+        v: FontSize,
+        parent: &Self,
+        device: &Device,
+    ) -> Option<NonNegativeLength> {
         let (adjusted_size, adjusted_unconstrained_size) =
             self.calculate_script_level_size(parent, device);
         // In this case, we have been unaffected by scriptminsize, ignore it

components/style/values/specified/font.rs

@@ -593,15 +593,25 @@ impl FontSize {
                 // Extract the ratio/offset and compose it
                 if (calc.em.is_some() || calc.percentage.is_some()) && parent.keyword_info.is_some() {
                     let ratio = calc.em.unwrap_or(0.) + calc.percentage.map_or(0., |pc| pc.0);
-                    // Compute it, but shave off the font-relative part (em, %)
+                    // Compute it, but shave off the font-relative part (em, %).
-                    // This will mean that other font-relative units like ex and ch will be computed against
+                    //
-                    // the old font even when the font changes. There's no particular "right answer" for what
+                    // This will mean that other font-relative units like ex and
-                    // to do here -- Gecko recascades as if the font had changed, we instead track the changes
+                    // ch will be computed against the old parent font even when
-                    // and reapply, which means that we carry over old computed ex/ch values whilst Gecko
+                    // the font changes.
-                    // recomputes new ones. This is enough of an edge case to not really matter.
+                    //
-                    let abs = calc.to_computed_value_zoomed(context, FontBaseSize::Custom(Au(0).into()))
+                    // There's no particular "right answer" for what to do here,
-                                  .length_component().into();
+                    // Gecko recascades as if the font had changed, we instead
-                    info = parent.keyword_info.map(|i| i.compose(ratio, abs));
+                    // track the changes and reapply, which means that we carry
+                    // over old computed ex/ch values whilst Gecko recomputes
+                    // new ones.
+                    //
+                    // This is enough of an edge case to not really matter.
+                    let abs = calc.to_computed_value_zoomed(
+                        context,
+                        FontBaseSize::InheritedStyleButStripEmUnits,
+                    ).length_component();
+
+                    info = parent.keyword_info.map(|i| i.compose(ratio, abs.into()));
                 }
                 let calc = calc.to_computed_value_zoomed(context, base_size);
                 calc.to_used_value(Some(base_size.resolve(context))).unwrap().into()

components/style/values/specified/length.rs

@@ -71,11 +71,17 @@ pub enum FontRelativeLength {
 /// A source to resolve font-relative units against
 #[derive(Clone, Copy, Debug, PartialEq)]
 pub enum FontBaseSize {
-    /// Use the font-size of the current element
+    /// Use the font-size of the current element.
     CurrentStyle,
-    /// Use the inherited font-size
+    /// Use the inherited font-size.
     InheritedStyle,
-    /// Use a custom base size
+    /// Use the inherited font-size, but strip em units.
+    ///
+    /// FIXME(emilio): This is very complex, and should go away.
+    InheritedStyleButStripEmUnits,
+    /// Use a custom base size.
+    ///
+    /// FIXME(emilio): This is very dubious, and only used for MathML.
     Custom(Au),
 }
 
@@ -85,6 +91,7 @@ impl FontBaseSize {
         match *self {
             FontBaseSize::Custom(size) => size,
             FontBaseSize::CurrentStyle => context.style().get_font().clone_font_size().size(),
+            FontBaseSize::InheritedStyleButStripEmUnits |
             FontBaseSize::InheritedStyle => context.style().get_parent_font().clone_font_size().size(),
         }
     }
@@ -99,36 +106,48 @@ impl FontRelativeLength {
         CSSPixelLength::new(pixel)
     }
 
-    /// Return reference font size. We use the base_size flag to pass a different size
+    /// Return reference font size.
-    /// for computing font-size and unconstrained font-size.
+    ///
-    /// This returns a pair, the first one is the reference font size, and the second one is the
+    /// We use the base_size flag to pass a different size for computing
-    /// unpacked relative length.
+    /// font-size and unconstrained font-size.
+    ///
+    /// This returns a pair, the first one is the reference font size, and the
+    /// second one is the unpacked relative length.
     fn reference_font_size_and_length(
         &self,
         context: &Context,
         base_size: FontBaseSize,
     ) -> (Au, CSSFloat) {
-        fn query_font_metrics(context: &Context, reference_font_size: Au) -> FontMetricsQueryResult {
+        fn query_font_metrics(
-            context.font_metrics_provider.query(context.style().get_font(),
+            context: &Context,
-                                                reference_font_size,
+            reference_font_size: Au,
-                                                context.style().writing_mode,
+        ) -> FontMetricsQueryResult {
-                                                context.in_media_query,
+            context.font_metrics_provider.query(
-                                                context.device())
+                context.style().get_font(),
+                reference_font_size,
+                context.style().writing_mode,
+                context.in_media_query,
+                context.device(),
+            )
         }
 
         let reference_font_size = base_size.resolve(context);
-
         match *self {
             FontRelativeLength::Em(length) => {
                 if context.for_non_inherited_property.is_some() {
-                    if matches!(base_size, FontBaseSize::CurrentStyle) {
+                    if base_size == FontBaseSize::CurrentStyle {
                         context.rule_cache_conditions.borrow_mut()
                             .set_font_size_dependency(
                                 reference_font_size.into()
                             );
                     }
                 }
-                (reference_font_size, length)
+
+                if base_size == FontBaseSize::InheritedStyleButStripEmUnits {
+                    (Au(0), length)
+                } else {
+                    (reference_font_size, length)
+                }
             },
             FontRelativeLength::Ex(length) => {
                 if context.for_non_inherited_property.is_some() {
@@ -182,7 +201,7 @@ impl FontRelativeLength {
                 // https://drafts.csswg.org/css-values/#rem:
                 //
                 //     When specified on the font-size property of the root
-                //     element, the rem units refer to the property’s initial
+                //     element, the rem units refer to the property's initial
                 //     value.
                 //
                 let reference_size = if context.is_root_element || context.in_media_query {