Closed Bug 1534494 Opened 5 years ago Closed 5 years ago

Try to bring some sanity into our font code.

Categories

(Core :: CSS Parsing and Computation, enhancement, P3)

enhancement

Tracking

()

RESOLVED FIXED
mozilla68
Tracking Status
firefox68 --- fixed

People

(Reporter: emilio, Assigned: emilio)

References

(Blocks 1 open bug)

Details

Attachments

(1 file)

It's not very easy to understand, and could be simpler if we centralized where it's handled.

These patches also will change how MathML script sizes are tracked when scriptlevel changes, and they have relative fonts in between. With this patch, any explicitly specified font-size is treated the same, being a scriptlevel boundary. Relative lengths always resolve relative to the constrained size.

This allows us to avoid the double font-size computation, and not give up on sanity with keyword font-sizes.

It's not very easy to understand on its current state, and it causes subtle bugs
like bug 1533654.

It could be simpler if we centralized where the interactions between properties
are handled. This patch does this.

This patch also changes how MathML script sizes are tracked when scriptlevel
changes and they have relative fonts in between.

With this patch, any explicitly specified font-size is treated the same (being a
scriptlevel boundary), regardless of whether it's either an absolute size, a
relative size, or a wide keyword.

Relative lengths always resolve relative to the constrained size, which allows
us to avoid the double font-size computation, and not give up on sanity with
keyword font-sizes.

I think given no other browser supports scriptlevel it seems like the right
trade-off.

I think this is probably fine, but given that it has some effect on how MathML is handled, I'd like to check with Frédéric in case he has any concerns.

Flags: needinfo?(fred.wang)

@Emilio: Can you explain what's the difference for MathML? If the scriptlevel is treated the same when there is no explicit user font-size changes, I suppose it should be ok.

Flags: needinfo?(fred.wang)

(In reply to Frédéric Wang (:fredw) from comment #3)

@Emilio: Can you explain what's the difference for MathML? If the scriptlevel is treated the same when there is no explicit user font-size changes, I suppose it should be ok.

Yes, that's right. This only changes the case of scriptlevel changes across explicit font-relative changes, see the only mathml test I had to modify, which simplified looks like:

<mstyle scriptlevel="0" scriptsizemultiplier="0.5" scriptminsize="18px" style="font-size:48px;">
  <mstyle scriptlevel="+2">
    <mstyle style="font-size:200%;">
      <mstyle scriptlevel="-1">
        <mi>Id</mi>
      </mstyle>
    </mstyle>
  </mstyle>
</mstyle>

Before that patch, we computed the font-relative units to both the constrained and unconstrained font sizes, which means that the computed font-sizes were:

Element Kind Constrained Unconstrained
mstyle explicit (48px) 48px 48px
mstyle > mstyle implicit (scriptlevel = +2) 18px (12px, but clamped to scriptminsize) 12px
mstyle > mstyle > mstyle explicit (200%) 36px 24px
mstyle > mstyle > mstyle > mstyle implicit (scriptlevel = -1) 48px 48px

With this change, any explicit font-size declaration acts as a boundary for scriptlevel constrainment, so it resets the unconstrained and constrained sizes, getting:

Element Kind Constrained Unconstrained
mstyle explicit (48px) 48px 48px
mstyle > mstyle implicit (scriptlevel = +2) 18px (12px, but clamped to scriptminsize) 12px
mstyle > mstyle > mstyle explicit (200%) 36px 36px
mstyle > mstyle > mstyle > mstyle implicit (scriptlevel = -1) 72px 72px

This is nicer IMO because:

  • It's simpler, i.e., no need to resolve explicit font-sizes twice (once against the constrained size, once against the unconstrained one).
  • It's easier to reason about, given it reduces the interactions between scriptlevel and font-size. No need to keep track of relative vs. absolute sizes, relative, absolute, and calc lengths behave the same. I had to use a debugger to figure out why we come up with the 48px right now.
  • Handles CSS wide-keywords in a sensible way (e.g. font-size: inherit and such), which we don't right now.

Anyhow, does that seem fine?

Assignee: nobody → emilio

Yes that sounds simpler to me, I would like MathML Core to specify something similar (even without minsize if possible). From a quick look at the MathML 3 version, I don't see anything that contradicts https://www.w3.org/Math/draft-spec/chapter3.html#presm.scriptlevel although to be honest I don't really understand this:

"The changes to the font size due to scriptlevel should be viewed as being imposed from ‘outside’ the node. This means that the effect of scriptlevel is applied before an explicit mathsize (see Section 3.2.2 Mathematics style attributes common to token elements) on a token child of mfrac. Thus, the mathsize effectively overrides the effect of scriptlevel. However, that change to scriptlevel changes the current font size, which affects the meaning of an "em" length (see Section 2.1.5.2 Length Valued Attributes) and so the scriptlevel still may have an effect in such cases."

Yeah, I don't understand what that piece of the spec wants to say, though generally this patch makes Firefox behavior match the spec a bit more closely (we used to apply scriptlevel changes before rather than after explicit font size changes).

Status: NEW → ASSIGNED
Priority: -- → P3
Pushed by ealvarez@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e05dc78eb427
Try to bring some more sanity into our font code. r=manishearth
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla68
Depends on: 1536718
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: