w3c / csswg-drafts

CSS Working Group Editor Drafts
https://drafts.csswg.org/
Other
4.52k stars 672 forks source link

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

Open Psychpsyo opened 5 days ago

Psychpsyo commented 5 days ago

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 commented 4 days ago

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

Psychpsyo commented 4 days ago

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 commented 4 days ago

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 commented 4 days ago

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 commented 4 days ago

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 commented 4 days ago

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 commented 4 days ago

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