Bug 196232

Summary: FontFace constructor throws an exception when there is a name which starts with a number
Product: WebKit Reporter: Ryosuke Niwa <rniwa>
Component: CSSAssignee: Myles C. Maxfield <mmaxfield>
Status: RESOLVED FIXED    
Severity: Normal CC: bfulgham, commit-queue, dino, koivisto, mmaxfield, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=196381
Attachments:
Description Flags
Reduction
none
Patch none

Description Ryosuke Niwa 2019-03-25 17:16:31 PDT
new FontFace('x 2y', 'url(abc.woff2)') throws an exception in Safari 12.2 but not in Chrome or Firefox.
Comment 1 Ryosuke Niwa 2019-03-25 20:37:47 PDT
Huh, Blink doesn't seem to do any validation for the font name at all :(
https://github.com/chromium/chromium/blob/master/third_party/blink/renderer/core/css/font_face.cc#L200
Comment 2 Myles C. Maxfield 2019-03-25 21:28:57 PDT
Oh, it's because there's a space in there.
Comment 3 Myles C. Maxfield 2019-03-25 21:40:24 PDT
This is because of the historical difference between font-family:foo; and font-family:”foo”;

FontFace::parseString() is passing its argument directly to consumeFontFamilyDescriptor() in CSSPropertyParser.cpp, which parses the string according to the first form. The first form doesn’t allow spaces.

We should adopt the Chrome/Firefox behavior, and the spec should be clarified here.
Comment 4 Ryosuke Niwa 2019-03-26 14:53:40 PDT
Created attachment 366004 [details]
Reduction

In Chrome & Firefox, this file would say "OK". In WebKit, it does not.
Comment 5 Radar WebKit Bug Importer 2019-03-26 14:54:06 PDT
<rdar://problem/49293978>
Comment 6 Ryosuke Niwa 2019-03-26 15:38:18 PDT
Relevant spec text:
https://drafts.csswg.org/css-font-loading/#font-face-constructor

> 1. ...
> Parse the family argument, and the members of the descriptors argument, according to > the grammars of the corresponding descriptors of the CSS @font-face rule
> ...

https://drafts.csswg.org/css-fonts-4/#font-face-rule
https://drafts.csswg.org/css-fonts-4/#font-family-desc
https://drafts.csswg.org/css-fonts-4/#family-name-value

Then finally,
https://drafts.csswg.org/css-fonts-4/#family-name-syntax
> Font family names other than generic families must either be given quoted as <string>s, or unquoted as a sequence of one or more identifiers.
> ...
> If a sequence of identifiers is given as a <family-name>, the computed value is the name converted to a string by joining all the identifiers in the sequence by single spaces.
Comment 7 Ryosuke Niwa 2019-03-26 15:39:09 PDT
We probably need to amend the following text for FontFace constructor:

https://drafts.csswg.org/css-font-loading/#font-face-constructor

> If any of them fail to parse correctly, reject font face’s [[FontStatusPromise]] with a DOMException named "SyntaxError", set font face’s corresponding attributes to the empty string, and set font face’s status attribute to "error". Otherwise, set font face’s corresponding attributes to the serialization of the parsed values.
Comment 8 Myles C. Maxfield 2019-03-28 13:55:01 PDT
https://github.com/w3c/csswg-drafts/issues/3776
Comment 9 Myles C. Maxfield 2019-03-28 17:20:52 PDT
Created attachment 366226 [details]
Patch
Comment 10 WebKit Commit Bot 2019-03-28 19:26:53 PDT
Comment on attachment 366226 [details]
Patch

Clearing flags on attachment: 366226

Committed r243637: <https://trac.webkit.org/changeset/243637>
Comment 11 WebKit Commit Bot 2019-03-28 19:26:55 PDT
All reviewed patches have been landed.  Closing bug.