Bug 246570 – font shorthand should not reject values that happen to have CSS-wide keywords as non-first identifiers in a font family name

• Status: RESOLVED
• Resolution: FIXED
• Component: CSS

[Tim Nguyen (:ntim)]

WPT: css/css-fonts/test_font_family_parsing.html

The code in CSSPropertyParser::consumeFont seems to assume a certain order for font-size.

[Darin Adler]

The failure I see in this test has nothing to do with order of font size. It's about whether the CSS-wide keywords are allowed as words in family names. The concatenateFamilyName function checks isValidCustomIdentifier, but probably should not be doing that.

[Darin Adler]

Reading CSS Fonts Module Level 4, it seems that the web platform test expectations are wrong, and do not match the specification. The specification says that family names that are not quoted are series of <custom-ident> and says that CSS-wide keywords are not valid <custom-ident>s. The default keyword is reserved and is also not a valid <custom-ident>.

[Tim Nguyen (:ntim)]

(In reply to Darin Adler from comment #1)
> The failure I see in this test has nothing to do with order of font size.
> It's about whether the CSS-wide keywords are allowed as words in family
> names. The concatenateFamilyName function checks isValidCustomIdentifier,
> but probably should not be doing that.

Ah, the `font-family` subtests threw me off.

> Reading CSS Fonts Module Level 4, it seems that the web platform test expectations are wrong, and do not match the specification. The specification says that family names that are not quoted are series of <custom-ident> and says that CSS-wide keywords are not valid <custom-ident>s. The default keyword is reserved and is also not a valid <custom-ident>.

The level 3 spec is more relaxed about the definition of <family-name>: https://www.w3.org/TR/css-fonts-3/#family-name-value 

Either way, it does seem like our handling is inconsistent and needs to be fixed, because we accept CSS-wide keywords in font-family as words in family names, but not as part of the `font` shorthand, only in the `font-family` longhand.

See consumeFamilyNameRaw in CSSPropertyParserHelpers.

[Radar WebKit Bug Importer]

<rdar://problem/101459948>

[Darin Adler]

for now, I suggest we ignore the CSS Fonts Module Level 4 use of <custom-ident>. We should probably file an issue to get the specification revised.

Use of an identifier as a family name should always be safe even if it is not a valid custom identifier. If it’s a context where a CSS-wide identifier is allowed then it will be caught as such before trying to parse the identifier. If it’s serialization we are concerned about, it will be quoted, and the fact that it was originally specified as an identifier will have no effect.

[Darin Adler]

Hmm, that’s not going to work. The css/css-fonts/test_font_family_parsing.html test in WPT requires that we continue to forbid some of these identifiers; I need to look more closely at what is going on.

[Darin Adler]

I figured out the real problem. It *is* specific to font, not font-family. There’s logic at the beginning of consumeFont that does an incorrect check for inherit and initial.

[Darin Adler]

Pull request: https://github.com/WebKit/WebKit/pull/5681

[EWS]

Committed 255894@main (3dde8827051a): <https://commits.webkit.org/255894@main>

Reviewed commits have been landed. Closing PR #5681 and removing active labels.