1396450 - Identifier in feature value definition of @font-feature-values should be case-sensitive

Status: RESOLVED FIXED

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

Description

[Xidorn Quan [:xidorn] UTC+10]

Currently, gfxFontFeatureValueSet::GetFontFeatureValuesFor and gfxFontFeatureValueSet::AppendFeatureValueHashEntry lowercase the name in feature value definition. However, the spec [1] says
> The identifiers used within feature value definitions follow
> the rules of CSS user identifiers and are case-sensitive.

The spec sounds reasonable. We should probably change our impl to match it.


[1] https://drafts.csswg.org/css-fonts-3/#basic-syntax

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Don't apply ToLowerCase to font feature identifiers used in @font-feature-values rules, as the spec says they're case sensitive

This looks like a straightforward spec compliance issue, we should just fix it. Note that we have an existing reftest that tests case-insensitivity of these identifiers, so we'll need to adjust that accordingly. Let's see what try thinks: https://treeherder.mozilla.org/#/jobs?repo=try&revision=515587f160a254c58c31f9fb665c3539fe53a6a3.

Comment 2

[Jonathan Kew [:jfkthame]]

Comment on attachment 8975319 [details] [diff] [review]
Don't apply ToLowerCase to font feature identifiers used in @font-feature-values rules, as the spec says they're case sensitive

The same testcase is present in both layout/reftests/font-features/ and testing/web-platform/tests/css/css-fonts/; we should really remove the copy from the layout/reftests suite as it is covered by WPT. However, this is just one of a bunch of tests that are duplicated in this way, so I'd prefer to deal with the de-duplication as a separate bug; here, I just applied the same adjustment to both copies.

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/4ee349d3078c
Don't apply ToLowerCase to font feature identifiers used in @font-feature-values rules, as the spec says they're case sensitive. r=xidorn

Comment 4

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Created web-platform-tests PR https://github.com/w3c/web-platform-tests/pull/10980 for changes under testing/web-platform/tests

Comment 5

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream web-platform-tests status checks passed, PR will merge once commit reaches central.

Comment 6

[Eliza Balazs [:ebalazs_]]

https://hg.mozilla.org/mozilla-central/rev/4ee349d3078c

Comment 7

[Web Platform Test Sync Bot [:wpt-sync] (Matrix: #interop:mozilla.org)]

Upstream PR merged