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-syntax-3] url( followed by comment(s) #10125

Closed
cdoublev opened this issue Mar 23, 2024 · 3 comments
Closed

[css-syntax-3] url( followed by comment(s) #10125

cdoublev opened this issue Mar 23, 2024 · 3 comments
Labels
Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered. css-syntax-3

Comments

@cdoublev
Copy link
Collaborator

cdoublev commented Mar 23, 2024

In Chrome and FF (at least), url(/**/"bg.jpg") is invalid, and url(/**/bg.jpg) serializes as url("/**/bg.jpg").

I could not find something equivalent to the following note from CSS2 in CSS Syntax 3:

COMMENT tokens do not occur in the grammar (to keep it readable), but any number of these tokens may appear anywhere outside other tokens.

Based on this note, I would expect url(/**/"bg.jpg") to be valid. However, according to this note (which may be usefull to reproduce in CSS Syntax 3, imo), also from CSS2, /**/bg.jpg must be seen as an invalid URL <ident>:

Note that COMMENT tokens cannot occur within other tokens: thus, "url(/*x*/pic.png)" denotes the URI "/*x*/pic.png", not "pic.png".

So I guess the first note does not apply anymore, ie. comment tokens can usually appear anywhere except in some places that you do not want to explicitly list.

And I cannot find any corresponding case on WPT.

If everything I said above is true and you think there is no need to clarify anything in CSS Syntax, this issue can be closed.

@dbaron
Copy link
Member

dbaron commented Mar 23, 2024

In CSS2, url("foo.jpg") was a single URL token. In css-syntax-3 it is parsed as a function and is three tokens. This is a substantive change; I don't remember when it was made, and it sounds like there may not be tests for it, and that implementations have not updated to match it.

@tabatkins
Copy link
Member

The change from the special url token to a standard function token was resolved on several years ago, if and only if the first substantive thing inside the parentheses is a string. Otherwise it continues to be parsed as the special-case url-token. (This allows us to handle additional arguments, which otherwise would require even more special parsing to keep in the url token syntax.)

Refreshing myself on the actual text, it appears that impls are in fact following the spec. After you see the url(, you scan forward until you stop seeing whitespace; if the next character is a " or ', you go ahead and emit the url( as a function token and parse as normal (leaving some whitespace behind, if needed, to ensure a ws token is emitted); otherwise you parse as a url token.

This means that, per spec, seeing the / that would normally start a comment actually triggers the "parse as a url token" behavior. And then in that algorithm, seeing a " is a parse error (it normally means you'd written something like url(foo"bar"), which is a mistake).

So yeah, impls match the spec. And I think the spec needs to stay as it is; comments are otherwise ambiguous syntax. In theory I could add more scan-ahead to see if the comment is closed and (after potentially more ws and comments) then I hit a quote character, but it would impose a little more parsing cost on impls to support a syntax that wasn't previous allowed anyway.

I no longer have an old impl around to test, but I suspect that url(/**/"foo") was probably always invalid, so the current behavior is consistent with that, too.

@tabatkins
Copy link
Member

So in conclusion: yes, the spec handles both of these cases and impls are correctly implementing that behavior, and I think that behavior is probably the best overall, given the legacy weirdnesses of url() syntax that we have to continue supporting.

@tabatkins tabatkins added the Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered. label Mar 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Closed as Question Answered Used when the issue is more of a question than a problem, and it's been answered. css-syntax-3
Projects
None yet
Development

No branches or pull requests

3 participants