Closed Bug 1434692 Opened 2 years ago Closed 2 years ago

calc() is not supported in font-feature-settings and font-variation-settings

Categories

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

58 Branch
defect

Tracking

()

RESOLVED FIXED

People

(Reporter: cnardi, Assigned: emilio)

Details

Attachments

(1 file)

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/65.0.3325.31 Safari/537.36

Steps to reproduce:

See https://w3c-test.org/css/css-values/calc-in-font-feature-settings.html


Actual results:

Using calc() in font-feature-settings resulted in an invalid property value


Expected results:

calc() should be supported in font-feature-settings as font-feature-settings takes an <integer> type per the spec.
Component: Untriaged → CSS Parsing and Computation
Product: Firefox → Core
Thanks for the report, you found one of the two places that get calc() wrong for numbers in non-internal properties, after an audit done right now, the other being `cursor`. :)
Assignee: nobody → emilio
Status: UNCONFIRMED → NEW
Ever confirmed: true
Jonathan, do you know why we use floats in font-variation-settings? The spec only seems to support <integer> in the syntax. Should I file a bug for this?

https://drafts.csswg.org/css-fonts-4/#propdef-font-variation-settings
Flags: needinfo?(jfkthame)
Huh, that's odd.... I think we need to raise it as a spec issue. Note that some variation fonts (e.g. Apple's Skia, which predates the OpenType 1.8 spec by many years) use a floating-point range where 1.0 is "normal", and values range from something like 0.6 - 3.2 for its "wght" axis, for example.

Also, while the spec says <integer>, it then goes on to say "Values are allowed to be fractional or negative." Ummmm.... how does that work, exactly?

One for the CSS WG, I think. Want to open an issue? Or I will, but not tonight...
Flags: needinfo?(jfkthame)
I think that was an editorial issue created by https://github.com/w3c/csswg-drafts/commit/cb2aec3d9003ca6c026e034e832d6df6ddff09cf. It looks like the changes in https://github.com/w3c/csswg-drafts/commit/152d48ea5a156db99baf0b07a3da36787fe2a244 were correct but in https://github.com/w3c/csswg-drafts/commit/cb2aec3d9003ca6c026e034e832d6df6ddff09cf any number value was renamed to be an integer. I'll leave a comment on https://github.com/w3c/csswg-drafts/issues/1127 to that effect.
(In reply to Chris Nardi from comment #4)
> I think that was an editorial issue created by
> https://github.com/w3c/csswg-drafts/commit/
> cb2aec3d9003ca6c026e034e832d6df6ddff09cf. It looks like the changes in
> https://github.com/w3c/csswg-drafts/commit/
> 152d48ea5a156db99baf0b07a3da36787fe2a244 were correct but in
> https://github.com/w3c/csswg-drafts/commit/
> cb2aec3d9003ca6c026e034e832d6df6ddff09cf any number value was renamed to be
> an integer. I'll leave a comment on
> https://github.com/w3c/csswg-drafts/issues/1127 to that effect.

Thanks!
Ugh, our font code was a mess. https://github.com/servo/servo/pull/19918 should fix this.
Summary: calc() is not supported in font-feature-settings → calc() is not supported in font-feature-settings and font-variation-settings
[ Triage 2017/02/20: P3 ]
Priority: -- → P3
Oh, this landed long time ago, sorry.

https://hg.mozilla.org/mozilla-central/rev/05d667efdfb8639bf21a4b93b9f641ab672b5b78
Status: NEW → RESOLVED
Closed: 2 years ago
Resolution: --- → FIXED
Comment on attachment 8947488 [details]
Add a WPT test for calc font-variation-settings.

Daniel Holbert [:dholbert] has approved the revision.

https://phabricator.services.mozilla.com/D544
Attachment #8947488 - Flags: review+
(In reply to Emilio Cobos Álvarez [:emilio] from comment #8)
> Oh, this landed long time ago, sorry.

(Looks like the test hadn't landed yet, because I'd totally missed the phabricator notification about the review request. Oops/sorry! r+)
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/d1a36af94ccd
Add a WPT test for calc font-variation-settings. r=dholbert
Pushed by ecoal95@gmail.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/1f571ca09860
followup: Mark the test as failing on the old style system. r=me
Created web-platform-tests PR https://github.com/w3c/web-platform-tests/pull/10009 for changes under testing/web-platform/tests
Upstream PR merged
You need to log in before you can comment on or make changes to this bug.