Bug 1355427 - Part 2: stylo: Support scriptlevel computation and scriptminsize; r?heycam draft
authorManish Goregaokar <manishearth@gmail.com>
Thu, 13 Apr 2017 13:25:01 +0800
changeset 565621 153d8e4dc47952434c33cf0087a0727b970ca4f9
parent 565620 0e63cf0184929f0b52da55a2c26b202fc253668a
child 565622 d65927dfe747b71696ff91053d6701a79b58b9a0
push id54926
push userbmo:manishearth@gmail.com
push dateThu, 20 Apr 2017 05:09:50 +0000
reviewersheycam
bugs1355427
milestone55.0a1
Bug 1355427 - Part 2: stylo: Support scriptlevel computation and scriptminsize; r?heycam scriptlevel is a property that affects how font-size is inherited. If scriptlevel is +1, for example, it will inherit as the script size multiplier times the parent font. This does not affect cases where the font-size is explicitly set. However, this transformation is not allowed to reduce the size below scriptminsize. If this inheritance will reduce it to below scriptminsize, it will be set to scriptminsize or the parent size, whichever is smaller (the parent size could be smaller than the min size because it was explicitly specified). Now, within a node that has inherited a font-size which was crossing scriptminsize once the scriptlevel was applied, a negative scriptlevel may be used to increase the size again. This should work, however if we have already been capped by the scriptminsize multiple times, this can lead to a jump in the size. For example, if we have text of the form: huge large medium small tiny reallytiny tiny small medium huge which is represented by progressive nesting and scriptlevel values of +1 till the center after which the scriptlevel is -1, the "tiny"s should be the same size, as should be the "small"s and "medium"s, etc. However, if scriptminsize kicked it at around "medium", then medium/tiny/reallytiny will all be the same size (the min size). A -1 scriptlevel change after this will increase the min size by the multiplier, making the second tiny larger than medium. Instead, we wish for the second "tiny" to still be capped by the script level, and when we reach the second "large", it should be the same size as the original one. We do this by cascading two separate font sizes. The font size (mSize) is the actual displayed font size. The unconstrained font size (mScriptUnconstrainedSize) is the font size in the situation where scriptminsize never applied. We calculate the proposed inherited font size based on scriptlevel and the parent unconstrained size, instead of using the parent font size. This is stored in the node's unconstrained size and will also be stored in the font size provided that it is above the min size. All of this only applies when inheriting. When the font size is manually set, scriptminsize does not apply, and both the real and unconstrained size are set to the explicit value. However, if the font size is manually set to an em or percent unit, the unconstrained size will be set to the value of that unit computed against the parent unconstrained size, whereas the font size will be set computing against the parent font size. MozReview-Commit-ID: 820BIWqno3L
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
servo/components/style/values/computed/length.rs
servo/components/style/values/specified/length.rs
--- a/servo/components/style/properties/gecko.mako.rs
+++ b/servo/components/style/properties/gecko.mako.rs
@@ -1347,21 +1347,180 @@ fn static_assert() {
     }
 
     // FIXME(bholley): Gecko has two different sizes, one of which (mSize) is the
     // actual computed size, and the other of which (mFont.size) is the 'display
     // size' which takes font zooming into account. We don't handle font zooming yet.
     pub fn set_font_size(&mut self, v: longhands::font_size::computed_value::T) {
         self.gecko.mFont.size = v.0;
         self.gecko.mSize = v.0;
+        self.gecko.mScriptUnconstrainedSize = v.0;
     }
-    pub fn copy_font_size_from(&mut self, other: &Self) {
-        self.gecko.mFont.size = other.gecko.mFont.size;
-        self.gecko.mSize = other.gecko.mSize;
+
+    /// 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: longhands::font_size::computed_value::T,
+                           parent: &Self) -> Option<Au> {
+        let (adjusted_size, adjusted_unconstrained_size)
+            = self.calculate_script_level_size(parent);
+        // In this case, we have been unaffected by scriptminsize, ignore it
+        if parent.gecko.mSize == parent.gecko.mScriptUnconstrainedSize &&
+           adjusted_size == adjusted_unconstrained_size {
+            self.set_font_size(v);
+            None
+        } else {
+            self.gecko.mFont.size = v.0;
+            self.gecko.mSize = v.0;
+            Some(Au(parent.gecko.mScriptUnconstrainedSize))
+        }
+    }
+
+    pub fn apply_unconstrained_font_size(&mut self, v: Au) {
+        self.gecko.mScriptUnconstrainedSize = v.0;
     }
+
+    /// Calculates the constrained and unconstrained font sizes to be inherited
+    /// from the parent.
+    ///
+    /// See ComputeScriptLevelSize in Gecko's nsRuleNode.cpp
+    ///
+    /// scriptlevel is a property that affects how font-size is inherited. If scriptlevel is
+    /// +1, for example, it will inherit as the script size multiplier times
+    /// the parent font. This does not affect cases where the font-size is
+    /// explicitly set.
+    ///
+    /// However, this transformation is not allowed to reduce the size below
+    /// scriptminsize. If this inheritance will reduce it to below
+    /// scriptminsize, it will be set to scriptminsize or the parent size,
+    /// whichever is smaller (the parent size could be smaller than the min size
+    /// because it was explicitly specified).
+    ///
+    /// Now, within a node that has inherited a font-size which was
+    /// crossing scriptminsize once the scriptlevel was applied, a negative
+    /// scriptlevel may be used to increase the size again.
+    ///
+    /// This should work, however if we have already been capped by the
+    /// scriptminsize multiple times, this can lead to a jump in the size.
+    ///
+    /// For example, if we have text of the form:
+    ///
+    /// huge large medium small tiny reallytiny tiny small medium huge
+    ///
+    /// which is represented by progressive nesting and scriptlevel values of
+    /// +1 till the center after which the scriptlevel is -1, the "tiny"s should
+    /// be the same size, as should be the "small"s and "medium"s, etc.
+    ///
+    /// However, if scriptminsize kicked it at around "medium", then
+    /// medium/tiny/reallytiny will all be the same size (the min size).
+    /// A -1 scriptlevel change after this will increase the min size by the
+    /// multiplier, making the second tiny larger than medium.
+    ///
+    /// Instead, we wish for the second "tiny" to still be capped by the script
+    /// level, and when we reach the second "large", it should be the same size
+    /// as the original one.
+    ///
+    /// We do this by cascading two separate font sizes. The font size (mSize)
+    /// is the actual displayed font size. The unconstrained font size
+    /// (mScriptUnconstrainedSize) is the font size in the situation where
+    /// scriptminsize never applied.
+    ///
+    /// We calculate the proposed inherited font size based on scriptlevel and
+    /// the parent unconstrained size, instead of using the parent font size.
+    /// This is stored in the node's unconstrained size and will also be stored
+    /// in the font size provided that it is above the min size.
+    ///
+    /// All of this only applies when inheriting. When the font size is
+    /// manually set, scriptminsize does not apply, and both the real and
+    /// unconstrained size are set to the explicit value. However, if the font
+    /// size is manually set to an em or percent unit, the unconstrained size
+    /// will be set to the value of that unit computed against the parent
+    /// unconstrained size, whereas the font size will be set computing against
+    /// the parent font size.
+    pub fn calculate_script_level_size(&self, parent: &Self) -> (Au, Au) {
+        use std::cmp;
+
+        let delta = self.gecko.mScriptLevel - parent.gecko.mScriptLevel;
+
+        let parent_size = Au(parent.gecko.mSize);
+        let parent_unconstrained_size = Au(parent.gecko.mScriptUnconstrainedSize);
+
+        if delta == 0 {
+            return (parent_size, parent_unconstrained_size)
+        }
+
+        /// XXXManishearth this should also handle text zoom
+        let min = Au(parent.gecko.mScriptMinSize);
+
+        let scale = (parent.gecko.mScriptSizeMultiplier as f32).powi(delta as i32);
+
+        let new_size = parent_size.scale_by(scale);
+        let new_unconstrained_size = parent_unconstrained_size.scale_by(scale);
+
+        if scale < 1. {
+            // The parent size can be smaller than scriptminsize,
+            // e.g. if it was specified explicitly. Don't scale
+            // in this case, but we don't want to set it to scriptminsize
+            // either since that will make it larger.
+            if parent_size < min {
+                (parent_size, new_unconstrained_size)
+            } else {
+                (cmp::max(min, new_size), new_unconstrained_size)
+            }
+        } else {
+            // If the new unconstrained size is larger than the min size,
+            // this means we have escaped the grasp of scriptminsize
+            // and can revert to using the unconstrained size.
+            // However, if the new size is even larger (perhaps due to usage
+            // of em units), use that instead.
+            (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.
+    pub fn inherit_font_size_from(&mut self, parent: &Self,
+                                  kw_inherited_size: Option<Au>) {
+        let (adjusted_size, adjusted_unconstrained_size)
+            = self.calculate_script_level_size(parent);
+        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.
+            // This only affects the use case of a generic font being used in MathML.
+            //
+            // If we were to fix this I would prefer doing it by removing the
+            // ruletree walk on the Gecko side in nsRuleNode::SetGenericFont
+            // and instead using extra bookkeeping in the mSize and mScriptUnconstrainedSize
+            // 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.mFont.size = adjusted_size.0;
+            self.gecko.mSize = adjusted_size.0;
+            self.gecko.mScriptUnconstrainedSize = adjusted_unconstrained_size.0;
+        } else if let Some(size) = kw_inherited_size {
+            // Parent element was a keyword-derived size.
+            self.gecko.mFont.size = size.0;
+            self.gecko.mSize = size.0;
+            // MathML constraints didn't apply here, so we can ignore this.
+            self.gecko.mScriptUnconstrainedSize = size.0;
+        } else {
+            // MathML isn't affecting us, and our parent element does not
+            // have a keyword-derived size. Set things normally.
+            self.gecko.mFont.size = parent.gecko.mFont.size;
+            self.gecko.mSize = parent.gecko.mSize;
+            self.gecko.mScriptUnconstrainedSize = parent.gecko.mScriptUnconstrainedSize;
+        }
+    }
+
     pub fn clone_font_size(&self) -> longhands::font_size::computed_value::T {
         Au(self.gecko.mSize)
     }
 
     pub fn set_font_weight(&mut self, v: longhands::font_weight::computed_value::T) {
         self.gecko.mFont.weight = v as u16;
     }
     ${impl_simple_copy('font_weight', 'mFont.weight')}
--- a/servo/components/style/properties/helpers.mako.rs
+++ b/servo/components/style/properties/helpers.mako.rs
@@ -262,57 +262,38 @@
                         % 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, 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});
+                                    longhands::font_size::cascade_specified_font_size(context,
+                                                                                      specified_value,
+                                                                                      computed,
+                                                                                      inherited_style.get_font());
                                 % else:
-                                context.mutate_style().mutate_${data.current_style_struct.name_lower}()
-                                                      .set_${property.ident}(computed ${maybe_wm});
+                                    % if property.has_uncacheable_values:
+                                    context.mutate_style().mutate_${data.current_style_struct.name_lower}()
+                                                          .set_${property.ident}(computed, cacheable ${maybe_wm});
+                                    % else:
+                                    context.mutate_style().mutate_${data.current_style_struct.name_lower}()
+                                                          .set_${property.ident}(computed ${maybe_wm});
+                                    % endif
                                 % endif
                             }
                             DeclaredValue::WithVariables(_) => unreachable!(),
                             DeclaredValue::CSSWideKeyword(keyword) => match keyword {
                                 % if not data.current_style_struct.inherited:
                                 CSSWideKeyword::Unset |
                                 % endif
                                 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(), 1.));
+                                        longhands::font_size::cascade_initial_font_size(context);
                                     % 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
@@ -323,21 +304,22 @@
                                 CSSWideKeyword::Inherit => {
                                     // This is a bit slow, but this is rare so it shouldn't
                                     // matter.
                                     //
                                     // FIXME: is it still?
                                     *cacheable = false;
                                     let inherited_struct =
                                         inherited_style.get_${data.current_style_struct.name_lower}();
-                                    context.mutate_style().mutate_${data.current_style_struct.name_lower}()
-                                        .copy_${property.ident}_from(inherited_struct ${maybe_wm});
+
                                     % if property.ident == "font_size":
-                                        context.mutate_style().font_size_keyword =
-                                            context.inherited_style.font_size_keyword;
+                                        longhands::font_size::cascade_inherit_font_size(context, inherited_struct);
+                                    % else:
+                                        context.mutate_style().mutate_${data.current_style_struct.name_lower}()
+                                            .copy_${property.ident}_from(inherited_struct ${maybe_wm});
                                     % endif
                                 }
                             }
                         }
                     }, error_reporter);
                 }
 
                 % if property.custom_cascade:
--- a/servo/components/style/properties/longhand/font.mako.rs
+++ b/servo/components/style/properties/longhand/font.mako.rs
@@ -408,16 +408,17 @@
             }
         }
     }
 </%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 properties::style_structs::Font;
     use std::fmt;
     use style_traits::ToCss;
     use values::{FONT_MEDIUM_PX, HasViewportPercentage};
     use values::specified::{FontRelativeLength, LengthOrPercentage, Length};
     use values::specified::{NoCalcLength, Percentage};
     use values::specified::length::FontBaseSize;
 
     impl ToCss for SpecifiedValue {
@@ -644,18 +645,18 @@
                 SpecifiedValue::Length(LengthOrPercentage::Length(ref l)) => {
                     l.to_computed_value(context)
                 }
                 SpecifiedValue::Length(LengthOrPercentage::Percentage(Percentage(value))) => {
                     base_size.resolve(context).scale_by(value)
                 }
                 SpecifiedValue::Length(LengthOrPercentage::Calc(ref calc)) => {
                     let calc = calc.to_computed_value(context);
-                    calc.length() +base_size.resolve(context)
-                                           .scale_by(calc.percentage())
+                    calc.length() + base_size.resolve(context)
+                                             .scale_by(calc.percentage())
                 }
                 SpecifiedValue::Keyword(ref key, fraction) => {
                     key.to_computed_value(context).scale_by(fraction)
                 }
                 SpecifiedValue::Smaller => {
                     FontRelativeLength::Em(0.85)
                         .to_computed_value(context, base_size)
                 }
@@ -705,16 +706,76 @@
         }
 
         match_ignore_ascii_case! {&*input.expect_ident()?,
             "smaller" => Ok(SpecifiedValue::Smaller),
             "larger" => Ok(SpecifiedValue::Larger),
             _ => Err(())
         }
     }
+
+    pub fn cascade_specified_font_size(context: &mut Context,
+                                      specified_value: &SpecifiedValue,
+                                      computed: Au,
+                                      parent: &Font) {
+        if let 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;
+        }
+
+        let parent_unconstrained = context.mutate_style()
+                                       .mutate_font()
+                                       .apply_font_size(computed,
+                                                        parent);
+
+        if let Some(parent) = parent_unconstrained {
+            let new_unconstrained = specified_value
+                        .to_computed_value_against(context, FontBaseSize::Custom(parent));
+            context.mutate_style()
+                   .mutate_font()
+                   .apply_unconstrained_font_size(new_unconstrained);
+        }
+    }
+
+    pub fn cascade_inherit_font_size(context: &mut Context, parent: &Font) {
+        // 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.style().font_size_keyword.map(|(kw, ratio)| {
+            SpecifiedValue::Keyword(kw, ratio).to_computed_value(context)
+        });
+        context.mutate_style().mutate_font()
+               .inherit_font_size_from(parent, kw_inherited_size);
+        context.mutate_style().font_size_keyword =
+            context.inherited_style.font_size_keyword;
+    }
+
+    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 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(), 1.));
+    }
 </%helpers:longhand>
 
 <%helpers:longhand products="gecko" name="font-size-adjust" animation_type="normal"
                    spec="https://drafts.csswg.org/css-fonts/#propdef-font-size-adjust">
     use std::fmt;
     use style_traits::ToCss;
     use values::HasViewportPercentage;
 
--- a/servo/components/style/properties/properties.mako.rs
+++ b/servo/components/style/properties/properties.mako.rs
@@ -1512,16 +1512,33 @@ pub mod style_structs {
                     // Corresponds to the fields in
                     // `gfx::font_template::FontTemplateDescriptor`.
                     let mut hasher: FnvHasher = Default::default();
                     hasher.write_u16(self.font_weight as u16);
                     self.font_stretch.hash(&mut hasher);
                     self.font_family.hash(&mut hasher);
                     self.hash = hasher.finish()
                 }
+
+                /// (Servo does not handle MathML, so this just calls copy_font_size_from)
+                pub fn inherit_font_size_from(&mut self, parent: &Self,
+                                              _: Au) {
+                    self.copy_font_size_from(parent);
+                }
+                /// (Servo does not handle MathML, so this just calls set_font_size)
+                pub fn apply_font_size(&mut self,
+                                       v: longhands::font_size::computed_value::T,
+                                       _: &Self,
+                                       _: &longhands::font_size::SpecifiedValue) {
+                    self.set_font_size(v);
+                }
+                /// (Servo does not handle MathML, so this does nothing)
+                pub fn apply_unconstrained_font_size(&mut self, _: Au) {
+                }
+
             % elif style_struct.name == "Outline":
                 /// Whether the outline-width property is non-zero.
                 #[inline]
                 pub fn outline_has_nonzero_width(&self) -> bool {
                     self.outline_width != ::app_units::Au(0)
                 }
             % elif style_struct.name == "Text":
                 /// Whether the text decoration has an underline.
@@ -2215,16 +2232,17 @@ pub fn apply_declarations<'a, F, I>(devi
                 LonghandId::TextDecorationLine |
                 LonghandId::WritingMode |
                 LonghandId::Direction
                 % if product == 'gecko':
                     | LonghandId::TextOrientation
                     | LonghandId::AnimationName
                     | LonghandId::TransitionProperty
                     | LonghandId::XLang
+                    | LonghandId::MozScriptLevel
                 % endif
             );
             if
                 % if category_to_cascade_now == "early":
                     !
                 % endif
                 is_early_property
             {
@@ -2286,22 +2304,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, fraction)) = inherited_style.font_size_keyword {
-                // Font size keywords will inherit as keywords and be recomputed
-                // each time.
+            } else {
+                // Font size must be explicitly inherited to handle keyword
+                // sizes and scriptlevel
                 let discriminant = LonghandId::FontSize as usize;
-                let size = PropertyDeclaration::FontSize(
-                    longhands::font_size::SpecifiedValue::Keyword(kw, fraction)
+                let size = PropertyDeclaration::CSSWideKeyword(
+                    LonghandId::FontSize, CSSWideKeyword::Inherit
                 );
                 (CASCADE_PROPERTY[discriminant])(&size,
                                                  inherited_style,
                                                  default_style,
                                                  &mut context,
                                                  &mut cacheable,
                                                  &mut cascade_info,
                                                  error_reporter);
--- a/servo/components/style/values/computed/length.rs
+++ b/servo/components/style/values/computed/length.rs
@@ -20,17 +20,17 @@ impl ToComputedValue for specified::NoCa
     type ComputedValue = Au;
 
     #[inline]
     fn to_computed_value(&self, context: &Context) -> Au {
         match *self {
             specified::NoCalcLength::Absolute(length) =>
                 length.to_computed_value(context),
             specified::NoCalcLength::FontRelative(length) =>
-                length.to_computed_value(context, /* base_size */ FontBaseSize::CurrentStyle),
+                length.to_computed_value(context, FontBaseSize::CurrentStyle),
             specified::NoCalcLength::ViewportPercentage(length) =>
                 length.to_computed_value(context.viewport_size()),
             specified::NoCalcLength::ServoCharacterWidth(length) =>
                 length.to_computed_value(context.style().get_font().clone_font_size())
         }
     }
 
     #[inline]
@@ -134,17 +134,16 @@ impl ToCss for CalcLengthOrPercentage {
         }
     }
 }
 
 impl ToComputedValue for specified::CalcLengthOrPercentage {
     type ComputedValue = CalcLengthOrPercentage;
 
     fn to_computed_value(&self, context: &Context) -> CalcLengthOrPercentage {
-
         let mut length = Au(0);
 
         if let Some(absolute) = self.absolute {
             length += absolute;
         }
 
         for val in &[self.vw.map(ViewportPercentageLength::Vw),
                      self.vh.map(ViewportPercentageLength::Vh),
@@ -155,17 +154,17 @@ impl ToComputedValue for specified::Calc
             }
         }
 
         for val in &[self.ch.map(FontRelativeLength::Ch),
                      self.em.map(FontRelativeLength::Em),
                      self.ex.map(FontRelativeLength::Ex),
                      self.rem.map(FontRelativeLength::Rem)] {
             if let Some(val) = *val {
-                length += val.to_computed_value(context, /* base_size */ FontBaseSize::CurrentStyle);
+                length += val.to_computed_value(context, FontBaseSize::CurrentStyle);
             }
         }
 
         CalcLengthOrPercentage {
             length: length,
             percentage: self.percentage,
         }
     }
--- a/servo/components/style/values/specified/length.rs
+++ b/servo/components/style/values/specified/length.rs
@@ -93,17 +93,17 @@ impl FontBaseSize {
             FontBaseSize::Custom(size) => size,
             FontBaseSize::CurrentStyle => context.style().get_font().clone_font_size(),
             FontBaseSize::InheritedStyle => context.inherited_style().get_font().clone_font_size(),
         }
     }
 }
 
 impl FontRelativeLength {
-    /// Computes the font-relative length. We use the inherited_size
+    /// Computes the font-relative length. We use the base_size
     /// flag to pass a different size for computing font-size and unconstrained font-size
     pub fn to_computed_value(&self, context: &Context, base_size: FontBaseSize) -> Au {
         fn query_font_metrics(context: &Context, reference_font_size: Au) -> FontMetricsQueryResult {
             context.font_metrics_provider.query(context.style().get_font(),
                                                 reference_font_size,
                                                 context.style().writing_mode,
                                                 context.in_media_query,
                                                 context.device)