1531250 - document.querySelector("div:not([data-val]") works, even though it shouldn't

Status: RESOLVED INVALID

(Core :: CSS Parsing and Computation, defect)

Description

[Pomax]

Ran into this because FF and Chrome "worked" but Safari kept erroring out, only to discover that Safari is probably doing the right thing and both FF and Chrome might have a bug in handling bad selectors. Specifically, this works:

document.querySelector("div:not([data-val]");

Note that this isn't a syntactically correct selector: it's missing a closing parenthesis for that not() pseudo. Amazingly, even the following clearly broken selectors work:

document.querySelector("div:not([data");

Both just... find a div. Rather than throwing the same kind of syntax error that Safari throws.

I tried looking in https://www.w3.org/TR/selectors-3/ for something that explains that inferring missing closing brackets and parens is fine, but couldn't find anything, so the fact that this even works at all in Firefox doesn't feel right: a bad selector shouldn't get accepted, it should be flagged as a bad selector.

Comment 1

[Emilio Cobos Álvarez (:emilio)]

https://drafts.csswg.org/css-syntax/#parse-error:

Certain points in the parsing algorithm are said to be parse errors. The error handling for parse errors is well-defined: user agents must either act as described below when encountering such problems, or must abort processing at the first error that they encounter for which they do not wish to apply the rules described below.

https://drafts.csswg.org/css-syntax/#consume-simple-block:

<EOF-token>
This is a parse error. Return the block.

(So the block is returned, and thus auto-closed)

Same happens on data:text/html,<style>:root { background: rgb(0, 255, 0, where all browsers do the right thing.

Comment 2

[Pomax]

Right, so according to the spec: that is explicitly a parse error.

Certainly, return the block as per the spec, but we should also be reporting that error to the user through the console (probably using a .warn since it's not fatal in FF). Even if our own browser can recover from it, users still need to know that the code they wrote is incorrect, so that they can fix their mistakes, because it will break in other browsers (most notably, Safari, the default browser for every Apple product out there).

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Well, or WebKit should fix it, right? :)

I'm ambivalent about adding a console warning for every error the CSS parser is able to recover from. We do output already a whole bunch of CSS errors if you enable CSS error reporting, but this happens at the tokenizer level, for which reporting the error and keep going is even harder.

Comment 4

[Pomax]

We might be, but I see zero indication of errors in this even though I've set dev tools to just show everything, so we have an opportunity to let developers know that they've written bad code that is guaranteed to break on the web, even if it doesn't break in their Firefox. Especially since in this particular case (string input to document.querySelector) there is nothing to lint for, automated tooling isn't going to tell you that the selector actually fails to parse, that's what we rely on runtime reporting for as developers.

I'd love for webkit to fix things, and I'll probably file a bug over on their tracker too, but in the mean time we have an opportunity to make sure sure the people who rely on Firefox for webdev are told about writing spec-technical erroneous code. It's on us to help people notice and fix bad code =)

Comment 5

[Emilio Cobos Álvarez (:emilio)]

I pointed out the WebKit bug in the See Also field, fwiw.

Comment 6

[Pomax]

Ah, missed that completely, thanks!