These three properties affect the font size (see ComputeScriptLevelSize and its callers in nsRuleNode.cpp). Should make most of the reftests from [1] pass. [1]: https://hg.mozilla.org/mozilla-central/raw-file/tip/layout/tools/reftest/reftest-analyzer.xhtml#logurl=https://queue.taskcluster.net/v1/task/dUbOmVtcSQiAVOZxXjjbMw/runs/0/artifacts/public/logs/live_backing.log&only_show_unexpected=1

Over to Fernando. Looks like this this affects quite a number of tests.

Already have a fix for this, didn't realize this was assigned. I've also fixed how inherit works, previously <span style="font-size: inherit; font-family: monospace;">b</span><span style="font-family: monospace;">c</span> would not give the same font size for both values.

Comment on attachment 8858199 [details] Bug 1355427 - Part 1: stylo: Support computing font-size against an arbitrary base size; https://reviewboard.mozilla.org/r/130144/#review132794 ::: servo/components/style/properties/longhand/font.mako.rs:658 (Diff revision 1) > use values::specified::length::FontRelativeLength; > match *self { > SpecifiedValue::Length(LengthOrPercentage::Length( > NoCalcLength::FontRelative(value))) => { > - value.to_computed_value(context, /* use inherited */ true) > + value.to_computed_value(context, > + Some(context.inherited_style().get_font().clone_font_size())) Can we use an enum like: ``` enum FontSizeBaseSize { CurrentStyle, InheritedStyle, Custom(Au), } ```

Manish, can you provide some detailed commit messages in these patches? I wasn't part of the conversation with dbaron explaining how these work, and I'm going to have to re-read and work out what nsRuleNode is doing wrt these. But for future code readers (and me when I come to review it), an explanation of why we need this functionality and what we're doing at a high level would be useful. Thanks.

Added both a commit message and a comment (identical)

Comment on attachment 8858199 [details] Bug 1355427 - Part 1: stylo: Support computing font-size against an arbitrary base size; https://reviewboard.mozilla.org/r/130144/#review134658 ::: servo/components/style/properties/longhand/font.mako.rs:657 (Diff revision 2) > SpecifiedValue::Length(LengthOrPercentage::Percentage(Percentage(value))) => { > - context.inherited_style().get_font().clone_font_size().scale_by(value) > + base_size.resolve(context).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() > + calc.length() +base_size.resolve(context) Nit: space after "+". ::: servo/components/style/values/computed/length.rs:28 (Diff revision 2) > 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, /* use inherited */ false), > + length.to_computed_value(context, /* base_size */ FontBaseSize::CurrentStyle), Nit: I guess given the name FontBaseSize we probably don't need that comment any more. (And below.) ::: servo/components/style/values/specified/length.rs:101 (Diff revision 2) > - /// Computes the font-relative length. We use the use_inherited flag to > - /// special-case the computation of font-size. > + /// Computes the font-relative length. We use the inherited_size > + /// flag to pass a different size for computing font-size and unconstrained font-size Should that be s/inherited_size/base_size/?

Comment on attachment 8858200 [details] Bug 1355427 - Part 2: stylo: Support scriptlevel computation and scriptminsize; https://reviewboard.mozilla.org/r/130146/#review134656 ::: commit-message-38c98:3 (Diff revision 3) > +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 Thanks for the awesome explanation! If it is identical to the comment, then I think it's fine to drop it from the commit message and just leave it in the comment. ::: servo/components/style/properties/gecko.mako.rs:1327 (Diff revision 3) > // 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. Please update this comment to briefly call out the third font size we're now setting. :-) ::: servo/components/style/properties/longhand/font.mako.rs:724 (Diff revision 3) > + if let SpecifiedValue::Keyword(kw, fraction) > + = *specified_value { Nit: maybe just on one line here? ::: servo/components/style/properties/properties.mako.rs:2154 (Diff revision 3) > % if product == 'gecko': > | LonghandId::TextOrientation > | LonghandId::AnimationName > | LonghandId::TransitionProperty > | LonghandId::XLang > + | LonghandId::MozScriptLevel Do -moz-script-size-multiplier and -moz-script-size-min-size need to be in the early property set too?

Comment on attachment 8859829 [details] Bug 1355427 - Part 3: stylo: Update test expectations; https://reviewboard.mozilla.org/r/131816/#review134696 Nice.

Comment on attachment 8859869 [details] Bug 1355427 - Part 1: stylo: Support computing font-size against an arbitrary base size; https://reviewboard.mozilla.org/r/131844/#review134704

Pushed by manishearth@gmail.com: https://hg.mozilla.org/integration/autoland/rev/1f1f3f34d558 Part 3: stylo: Update test expectations; r=heycam

Pushed by cmccormack@mozilla.com: https://hg.mozilla.org/integration/autoland/rev/27507fdcbacd Expectations adjustment followup. r=me

https://hg.mozilla.org/mozilla-central/rev/1f1f3f34d558 https://hg.mozilla.org/mozilla-central/rev/27507fdcbacd