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

[mediaqueries] Spec has conflicting info on whether or not <media-query-list> allows trailing commas #11254

Open
Psychpsyo opened this issue Nov 21, 2024 · 7 comments

Comments

@Psychpsyo
Copy link
Contributor

The railroad diagram in 2.1. Combining Media Queries does not allow a trailing comma:
image

But later on, in 3. Syntax, it says "To parse a <media-query-list> production, parse a comma-separated list of component values".
And parsing a comma-separated list of component values does allow a trailing comma.

I'd assume the trailing comma is allowed, especially since the railroad diagram is called out as "informal", but from what I can tell, it is still in a normative part of the spec.

(I haven't yet checked how browsers actually implement this, so that guess is based purely on the spec.)

@cdoublev
Copy link
Collaborator

An invalid <media-query> is then replaced by not all, as defined in § 3.2. Error Handling. So all, becomes all, not all.

@Psychpsyo
Copy link
Contributor Author

Psychpsyo commented Nov 21, 2024

An invalid <media-query> is then replaced by not all, as defined in § 3.2. Error Handling. So all, becomes all, not all.

Ah, but passing all, instead of all, (notice the whitespace) into parse a comma-separated list of component values would return a list with just all in it.

Also, in the section on error handling it says that "a grammar mismatch does not wipe out an entire media query list, just the problematic media query" so even if passing all, , the first media query in that still works.
So the trailing comma is still syntactically allowed, while not in the railroad diagram.

@cdoublev
Copy link
Collaborator

The trailing whitespace in my example was unintentional, sorry. It does not make any difference: matchMedia('all, ').media === matchMedia('all,').media is true. The last list item is either a whitespace token or empty, which then does not make a difference when parsing against the grammar (<media-query>).

@Psychpsyo
Copy link
Contributor Author

Psychpsyo commented Nov 21, 2024

The trailing whitespace in my example was unintentional, sorry. It does not make any difference: matchMedia('all, ').media === matchMedia('all,').media is true. The last list item is either a whitespace token or empty, which then does not make a difference when parsing against the grammar (<media-query>).

Without whitespace, the last item, as per my understanding of the spec, should be all, not empty:
Since parse a comma-separated list of component values will, while input is not empty:

  1. Consume a list of component values from input, with <comma-token> as the stop token, and append the result to groups. (This consumes all and adds it to the list to be returned)
  2. Discard a token from input. (This discards the trailing comma)
  3. Now input is empty, so the list containing only an all will be returned.

Note that both Firefox and Chrome don't do it like this, so I'm either misunderstanding something or this is a spec or browser bug.

Meanwhile, the railroad diagram makes it seem like all, should not be parseable as a <media-query-list> at all, since after circling back around through the comma, there is no further media query to be matched.

@cdoublev
Copy link
Collaborator

cdoublev commented Nov 21, 2024

Ah yes, this seems to be a regression introduced in 6ab5888 (related: #8834). It is not present in the last published version of the spec, linked in your initial comment.

This algo is currently only used to parse a list of media queries, of selectors in :is()/:where(), and font sources in @font-face/src. The current version of Chrome and Firefox accept a trailing comma in all these places.

Related tests:

Simple fix:

- 2. [=Discard a token=] from |input|.
+ 2. If the current input code point is <<comma-token>>,
+    [=discard a token=] from |input|,
+    and repeat step 1.

Possibly simpler:

  1. Normalize input, and set input to the result.
  2. Let groups be an empty list.
  3. If input is empty, return groups.
  4. While the next input code point is <comma-token>:
    1. Consume the next input code point
    2. Consume a list of component values from input,
      with <comma-token> as the stop token,
      and append the result to groups.

@Psychpsyo
Copy link
Contributor Author

Ah yes, this seems to be a regression introduced in 6ab5888 (related: #8834). It is not present in the last published version of the spec, linked in your initial comment.

Right, I did not read the old version of the spec closely enough.
And I guess it makes sense that the railroad diagram wouldn't be able to encapsulate the fact that it can still take the path through media query, even if it is invalid.

I got it now.
But I still think there should also be something that marks the railroad diagrams as non-normative in that case.
Except if diagrams are always non-normative?

@cdoublev
Copy link
Collaborator

It should probably be marked as non-normative. I agree.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants