1611589 - Incorrect precedence of font features vs letter-spacing

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect, P3)

Description

[Jonathan Kew [:jfkthame]]

Testcase:
data:text/html,<div style="font: 24px DejaVu Serif; letter-spacing: 10px; font-variant: common-ligatures">office

This renders with an "ffi" ligature in Firefox, even though the presence of letter-spacing should cause common ligatures to be disabled, according to the spec:

General and font specific font feature property settings are resolved in the order below, in ascending order of precedence. This ordering is used to construct a combined list of font features that affect a given text run.

1. Font features enabled by default, including features required for a given script.
2. If the font is defined via an @font-face rule, the font features implied by the font-feature-settings descriptor in the @font-face rule.
3. Font features implied by the value of the ‘font-variant’ property, the related ‘font-variant’ subproperties and any other CSS property that uses OpenType features (e.g. the ‘font-kerning’ property).
4. Feature settings determined by properties other than ‘font-variant’ or ‘font-feature-settings’. For example, setting a non-default value for the ‘letter-spacing’ property disables common ligatures.
5. Font features implied by the value of ‘font-feature-settings’ property.

We implement the disabling of ligatures due to non-default letter-spacing, but we do this too early in feature resolution, and the font-variant: common-ligatures property then overrides it. Only font-feature-settings should be able to do this.

As a result, we fail several of the examples in web-platform/tests/css/css-fonts/font-feature-resolution-001.html.

Comment 1

[Jonathan Kew [:jfkthame]]

Fixing this will require us to distinguish between feature settings in gfxFontStyle that originate from "high-level" controls like font-variant vs those from the low-level font-feature-settings property. Currently nsFont::AddFontFeaturesToStyle puts them all into the same featureSettings list, and we can't tell the difference.

Rather than separating them into two lists, which would bloat every gfxFontStyle instance (even though both lists would usually be empty), I propose to insert a delimiter between the high-level and low-level settings in the featureSettings array (if non-empty). Then when gfxFontShaper::MergeFontFeatures combines these with features coming from the @font-face rule, etc., it will be able to handle the two levels appropriately and slot ligature-disabling, if needed, in at the correct stage.

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1611589 - Fix precedence of font-feature-settings vs disabling of ligatures due to letter-spacing. r=lsalzman

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/71927edae8a3
Fix precedence of font-feature-settings vs disabling of ligatures due to letter-spacing. r=lsalzman

Comment 4

[Cristina Coroiu [:ccoroiu]]

https://hg.mozilla.org/mozilla-central/rev/71927edae8a3