Bug 163546 - CSS font-variation-settings should not handle uppercase axis names in variable fonts
Summary: CSS font-variation-settings should not handle uppercase axis names in variabl...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Text (show other bugs)
Version: WebKit Nightly Build
Hardware: Mac macOS 10.12
: P2 Normal
Assignee: Myles C. Maxfield
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2016-10-17 09:20 PDT by Laurence Penney
Modified: 2016-10-19 17:41 PDT (History)
8 users (show)

See Also:


Attachments
Patch (3.24 KB, patch)
2016-10-19 13:59 PDT, Myles C. Maxfield
no flags Details | Formatted Diff | Diff
Archive of layout-test-results from ews121 for ios-simulator-wk2 (19.29 MB, application/zip)
2016-10-19 14:58 PDT, Build Bot
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Laurence Penney 2016-10-17 09:20:58 PDT
When using CSS to set axis positions in variable fonts, only lowercase axis names function correctly.

Although the spec advises that both uppercase and lowercase axis names are allowed, and that there is no equivalence between uppercase and lowercase axes, in practice axes with uppercase names are ignored. Inspecting the DOM reveals that upper-to-lowercase conversion is applied to font-variation-settings as the value enters the DOM.

Two problematic fonts are BuffaloGalRegular.ttf (axes: CK,FR,HV,CN,BR,TC) and ZyconRegular.ttf (axes: T1,T2,T3,T4,M1,M2). These do not work in Webkit Nightly. A DOM inspection reveals the styles are case converted. For example:

style="font-variation-settings: 'CK  ' 0, 'FR  ' -0.298, 'HV  ' -0.06, 'CN  ' 0, 'BR  ' 0, 'TC  ' 0;"

… becomes …

style="font-variation-settings: 'ck ' 0, 'fr ' -0.298, 'hv ' -0.06, 'cn ' 0, 'br ' 0, 'tc ' 0;"

After this conversion, the axis names no longer match those in the font, and the axes do not function.

(Note also the replacement of two spaces by one, but that seems not to be a problem.)

Both BuffaloGalRegular.ttf and ZyconRegular.ttf function correctly after converting tag names to lowercase using TTX.

Spec: https://www.microsoft.com/typography/otspec/fvar.htm
Comment 1 Radar WebKit Bug Importer 2016-10-17 15:35:40 PDT
<rdar://problem/28810114>
Comment 2 Myles C. Maxfield 2016-10-19 13:48:00 PDT
Where can I get a copy of these fonts to test them out?
Comment 3 Myles C. Maxfield 2016-10-19 13:59:31 PDT
Created attachment 292108 [details]
Patch
Comment 4 Build Bot 2016-10-19 14:58:01 PDT
Comment on attachment 292108 [details]
Patch

Attachment 292108 [details] did not pass ios-sim-ews (ios-simulator-wk2):
Output: http://webkit-queues.webkit.org/results/2326585

New failing tests:
media/controls/ipad/close-page-with-picture-in-picture-video-assertion-failure.html
Comment 5 Build Bot 2016-10-19 14:58:05 PDT
Created attachment 292115 [details]
Archive of layout-test-results from ews121 for ios-simulator-wk2

The attached test failures were seen while running run-webkit-tests on the ios-sim-ews.
Bot: ews121  Port: ios-simulator-wk2  Platform: Mac OS X 10.11.6
Comment 6 Dean Jackson 2016-10-19 17:11:27 PDT
Comment on attachment 292108 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=292108&action=review

> Source/WebCore/css/parser/CSSParser.cpp:10596
> -        tag[i] = toASCIILower(character);
> +        tag[i] = character;

Did you do the same in the new parser code?

> LayoutTests/fast/text/variations/case-axis-names.html:7
> +This test passes if capitalized and lowercased axis names are distinct.
> +<div style="font: 100px '-apple-system'; font-variation-settings: 'WGHT' 1.8;">Hello</div>

Am I confused? The -expected version of this test doesn't use any variation, so what is it testing?
Comment 7 Myles C. Maxfield 2016-10-19 17:16:08 PDT
Comment on attachment 292108 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=292108&action=review

>> LayoutTests/fast/text/variations/case-axis-names.html:7
>> +<div style="font: 100px '-apple-system'; font-variation-settings: 'WGHT' 1.8;">Hello</div>
> 
> Am I confused? The -expected version of this test doesn't use any variation, so what is it testing?

The test is testing that "wght" is different from "WGHT". The font only responds to "wght" so if you specify "WGHT" it should be the same as specifying nothing.
Comment 8 Dean Jackson 2016-10-19 17:18:15 PDT
Comment on attachment 292108 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=292108&action=review

>>> LayoutTests/fast/text/variations/case-axis-names.html:7
>>> +<div style="font: 100px '-apple-system'; font-variation-settings: 'WGHT' 1.8;">Hello</div>
>> 
>> Am I confused? The -expected version of this test doesn't use any variation, so what is it testing?
> 
> The test is testing that "wght" is different from "WGHT". The font only responds to "wght" so if you specify "WGHT" it should be the same as specifying nothing.

Ah ok. Can you change the title of the bug to be "should not" rather than "does not"?
Comment 9 Myles C. Maxfield 2016-10-19 17:19:10 PDT
Comment on attachment 292108 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=292108&action=review

>> Source/WebCore/css/parser/CSSParser.cpp:10596
>> +        tag[i] = character;
> 
> Did you do the same in the new parser code?

https://bugs.webkit.org/show_bug.cgi?id=163712
Comment 10 WebKit Commit Bot 2016-10-19 17:41:53 PDT
Comment on attachment 292108 [details]
Patch

Clearing flags on attachment: 292108

Committed r207581: <http://trac.webkit.org/changeset/207581>
Comment 11 WebKit Commit Bot 2016-10-19 17:41:57 PDT
All reviewed patches have been landed.  Closing bug.