[css-font-loading-3] Unclear how to parse font families

[litherum]

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

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/#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.

So what if you have

new FontFace("4m", source)

It is neither a quoted string nor an unquoted sequence of one or more identifiers (identifiers can't start with a number), so WebKit throws an exception here. However, Chrome and Firefox treat the argument as if it was quoted.

[rniwa]

In fact, we're seeing web compatibility issues with the current WebKit's behavior to throw in such a case so we need to change the spec.

[litherum]

Also, looks like if the family name has commas, Firefox replaces the entire string with the empty string, WebKit throws an exception, and Chrome honors the comma.

[litherum]

I'm modifying WebKit to match Chrome.

[litherum]

https://bugs.webkit.org/show_bug.cgi?id=196232

[emilio]

cc @heycam

Looks like Firefox throws if there are commas from the family setter, but just bail out and leave the empty string from the constructor, that looks wrong.

[emilio]

Chrome behavior for the comma case doesn't make sense, at the very least it should serialize it quoted, if it is indeed treating it as a single font family.

[emilio]

I'd be happy to fix Firefox to properly throw on construction for invalid families...

Though that being said, given it breaks websites, does it make sense to spec this as something like: "If the value doesn't conform to the family name syntax, then treat it as a quoted family name"?

We definitely should serialize it quoted in that case though, IMO.

[tabatkins]

[issue filed 23 hours ago]

[comment posted 20 hours ago]

I'm modifying WebKit to match Chrome.

Dang Myles, wanna give the rest of us a moment to catch up first? ^_^

---

Is there any reason to invoke the CSS parser for the family name at all? We could just interpret the string as a family name directly, no need for escapes or anything like that.

[litherum]

@tabatkins that's what my patch does.

After my patch, Chrome and WebKit both:

A) For the FontFace constructor and .family property, simply retain and return the given string.
B) For the CSSOM HTMLStyleElement.sheet.rules[n].style.getPropertyValue(font-family) continue running the string through isCSSTokenizerIdentifier(string) ? string : serializeString(string). This will put quotes around the string and do some substitutions if the string isn't a valid ident.

[litherum]

@emilio the grammar for the font-family descriptor (as opposed to the font-family property) doesn't accept a list. It's just a single value <family-name>.

[emilio]

Right, that's why I think returning just the input string is bogus. But if we're giving up on doing font-family parsing on it I guess it's fine. It just seems a bit weird to have FontFace returning a different string from CSSFontFaceRule.

[tabatkins]

@emilio The only difference is that FontFace implicitly assumes that what it's been passed is a literal string, and will reflect its contents directly, while the associated CSSFontFaceRule will reflect it as a string token if necessary, right?

That is, constructing a FontFace("4em") will have FontFace.family return "4em", while the corresponding CSSFontFaceRule.fontFamily returns '"4em"', right? But FontFace("foo bar") will return "foo bar" for both.

[tabatkins]

Okay, so it sounds like we've agreed that FontFace should basically assume the argument is the contents of a CSS string? Should we do any CSS parsing (to handle CSS escapes in the string) or just take it literally? (In other places where we do something like this, we just take the JS string literally and do no parsing; if you then ask for the value via a CSS API, it might come with some CSS escapes as a result.)

[litherum]

Did we ever get a resolution on this? I'd like to move forward with this spec change.