1435983 - The font-variations-setting property should be reset by the font shorthand

Status: RESOLVED FIXED

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

Description

[Jonathan Kew [:jfkthame]]

Currently, the font shorthand does not affect font-variation-settings. This was an oversight when support was originally implemented in bug 1321022 (though I'm not sure whether it was explicitly mentioned in any spec yet at that time).

Anyway, we should fix it now: the font shorthand should reset font-variation-settings to 'normal'.

https://drafts.csswg.org/css-fonts-4/#font-prop confirms this is the correct behavior.

Comment 1

[jason]

I may be overstating something that's clear to others already, but I think it's important to point out that with this behavior, the font shorthand should only override that specific property in the font-variation-settings. This is crucial because there will always be variation types that do not have a shorthand equivalent (such as grade or optical size). So it will often be the case that someone has to use:

font-variation-settings: "OPSZ" 16;
font-weight: 575;

This should result in _both_ OPSZ and weight being set as specified.

If someone sets:

font-variation-settings: "OPSZ" 16, "wght" 400;
font-weight: 575;

Then the OPSZ value should still be set to 16, and the weight to 575

I hope I'm explaining that clearly enough.

Comment 2

[Jonathan Kew [:jfkthame]]

That's a separate issue -- the interaction between higher-level properties like font-weight (also font-stretch, font-style) and font-variation-settings. We don't have those hooked up yet; font-weight & friends are used for font *selection* but do not then customize variation settings within the selected font. (Yet.)

This bug is about the 'font' shorthand itself: i.e. things like "font: bold 12px sans-serif". That should reset the other sub-properties of font-*, including font-variation-settings, to their initial values.

So for example

  <div style="font-variation-settings: 'wght' 999">foo <span style="font: 12px serif">bar

should NOT apply the "font-variation-settings: 'wght' 999" property to the span, because its (shorthand) font property resets it. But currently we fail to do that properly.

Regarding your examples (in the future, when properties like font-weight are connected to variation axes):

> font-variation-settings: "OPSZ" 16;
> font-weight: 575;
>
> This should result in _both_ OPSZ and weight being set as specified.

Correct, in this case both those settings would apply.

> If someone sets:
> 
> font-variation-settings: "OPSZ" 16, "wght" 400;
> font-weight: 575;
> 
> Then the OPSZ value should still be set to 16, and the weight to 575

No, here the weight axis will be set to 400, because the low-level font-variation-settings property takes precedence over variation values that are implied by higher-level properties. See https://drafts.csswg.org/css-fonts-4/#feature-variation-precedence.

(But again, that's not part of _this_ bug, which is about the interaction of the 'font' shorthand and the 'font-variation-settings' longhand property. Or rather, the failure to implement that interaction.)

Comment 3

[jason]

Understood - thanks for the clarification! The first case I mentioned was the most important one, but I also reread the spec and understand better now what the differentiation is.

Comment 4

[Jet Villegas (inactive)]

[ Triage 2017/02/20: P3 ]

Comment 5

[Jonathan Kew [:jfkthame]]

Attachment: Make the 'font' shorthand reset the 'font-variation-settings' property, as required by css-fonts-4

Here's the gecko side of this; I have just opened https://github.com/servo/servo/pull/20191 for the servo change.

Comment 6

[Emilio Cobos Álvarez (:emilio)]

Comment on attachment 8955882 [details] [diff] [review]
Make the 'font' shorthand reset the 'font-variation-settings' property, as required by css-fonts-4

Review of attachment 8955882 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, but presumably also needs a similar change to layout/style/test/test_system_font_serialization.html for the test to pass with the font variations pref enabled, right?

Not a big deal because the tree will be green (given the pref is off), but I'd rather change it now than having to change it when the pref is flipped.

Comment 7

[Jonathan Kew [:jfkthame]]

Attachment: Make the 'font' shorthand reset the 'font-variation-settings' property, as required by css-fonts-4

Good catch, thanks -- updated the patch to include the test_system_font_serialization tweak; carrying over r=emilio. We'll need to wait for the servo change to land first, then this can follow.

Comment 8

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a6cc8bd35279
Make the 'font' shorthand reset the 'font-variation-settings' property, as required by css-fonts-4. r=emilio

Comment 9

[Sebastian Hengst [:aryx] (needinfo me if it's about an intermittent or backout)]

https://hg.mozilla.org/mozilla-central/rev/a6cc8bd35279