Bug 163546

Summary: CSS font-variation-settings should not handle uppercase axis names in variable fonts
Product: WebKit Reporter: Laurence Penney <lorp>
Component: TextAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, dino, jonlee, lorp, mmaxfield, thorton, webkit-bug-importer, zalan
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Mac   
OS: macOS 10.12   
Attachments:
Description Flags
Patch
none
Archive of layout-test-results from ews121 for ios-simulator-wk2 none

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.