Closed Bug 1434692 Opened 2 years ago Closed 2 years ago
calc() is not supported in font-feature-settings and font-variation-settings
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
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...
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 firstname.lastname@example.org: https://hg.mozilla.org/integration/mozilla-inbound/rev/d1a36af94ccd Add a WPT test for calc font-variation-settings. r=dholbert
Pushed by email@example.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.