Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[css-fonts] Confirmation/clarification re parsing the @font-face src: descriptor #8183

Open
jfkthame opened this issue Dec 3, 2022 · 4 comments
Labels
css-fonts-4 Current Work

Comments

@jfkthame
Copy link
Contributor

jfkthame commented Dec 3, 2022

The CSS Fonts section on Parsing the 'src' descriptor says that, having parsed the comma-separated list of component values:

If a component value is parsed correctly and is of a font format or font tech that the UA supports, add it to the list of supported sources. If parsing a component value results in a parsing error or its format or tech are unsupported, do not add it to the list of supported sources.

If there are no supported entries at the end of this process, the value for the src descriptor is a parse error.

As I understand this, the intention is that if any individual component of the comma-separated src list cannot be parsed, that component (only) is to be omitted from the list of sources, but this does not cause the src descriptor as a whole to be rejected.

This behavior is important for future extensibility: e.g. if a future version of CSS adds new font format or technology keywords, those will not be recognized as valid arguments to the format() and tech() functions by existing UAs. Therefore, a src component that specifies such a format or technology would give a parsing error in older UAs. The component should then be ignored as unsupported, but we do not want this to cause the entire src descriptor to be dropped.

It appears that current browsers do not implement this behavior correctly; if they encounter a parse error in a component of the list, they treat the entire src descriptor as invalid.

At https://codepen.io/jfkthame/pen/jOKedQm I've put a bunch of examples of @font-face rules with bad components in the src list. If my understanding of the spec (and the desirable behavior) is correct, all the test lines there should render with the same font as the reference line; but both Firefox and Chrome currently fail all the test lines, and Safari fails all but two of them (it seems to ignore invalid keywords in the format() function).

I'm intending to fix this behavior in Gecko, unless people fundamentally disagree with my reading of the spec here. I also wonder if the spec should include an explicit example showing an invalid component in the src list and clarifying that other valid components are still to be used.

cc: @drott @litherum @svgeesus

@litherum
Copy link
Contributor

litherum commented Dec 3, 2022

Yes, your understanding is entirely correct. You're also right that spec does not match reality, and that reality should change to match the spec (rather than vice-versa). The motivations for the spec text are exactly as you describe them.

I've just never gotten around to updating WebKit's implementation to match the spec. I do plan to do it at some point, though.

@jfkthame
Copy link
Contributor Author

jfkthame commented Dec 4, 2022

Thanks, Myles.

So this means that some of the existing WPT tests, such as https://github.com/web-platform-tests/wpt/blob/4990362322013386a4586e46dc62d72745be6d71/css/css-fonts/parsing/font-face-src-format.html#L49-L54, are incorrect as currently written. They assert that a component with a parse error (like format(xyzz 200px)) invalidates the entire src descriptor; but those tests also include a valid component in the list, and so the descriptor as a whole is still valid. Only the bad component should be dropped.

@svgeesus svgeesus added the css-fonts-4 Current Work label Dec 7, 2022
@jfkthame
Copy link
Contributor Author

jfkthame commented Dec 8, 2022

FTR, in web-platform-tests/wpt#37348, I recently updated tests in css/css-fonts/parsing to conform to this understanding of the spec.

@cdoublev
Copy link
Collaborator

cdoublev commented Dec 8, 2022

I wonder if these tests are not biased because whether multiple src declarations are valid seems ambiguous (see #7753).

EDIT: nevermind, the previous valid test case has a different tech().

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
css-fonts-4 Current Work
Projects
None yet
Development

No branches or pull requests

4 participants