261332 - Line number for Error: Selector expected. Ruleset ignored due to bad selector. caused by trailing comma is tied to { instead of ,

Status: NEW

(Core :: CSS Parsing and Computation, defect)

Description

[timeless]

Error: Selector expected.  Ruleset ignored due to bad selector.
Source File: data:text/html,<style>..ignorethiserror /*which is on line 1*/ {}
foo,/*error should be on line 1*/%0d%0d%0d{ /* not line 4*/}</style>
Line: 4

Comment 1

[Hixie (not reading bugmail)]

Actually any line would be correct for this error. While the start of the
selector is on line 1, the ignored ruleset is on lines 1 to 4 inclusive, and the
declaration block (which is the end of the selector) starts on line 4.

Comment 2

[timeless]

i know, but ideally the report would complain about the line with the error, not
the place where the css was ignored.

Comment 3

[Boris Zbarsky [:bzbarsky]]

The basic issue here is that by the time we catch the error we've read the '{'
(which we have to do to realize there is an error) and advanced the scanner to
the next line.  Then we unget the token, but when we report the error we report
the current scanner line number, not the line number associated with mToken.

One way to deal with this may be to store line numbers in tokens and to use the
line from mToken when we have pushback.  But that sounds a little painful...

Comment 4

[Hixie (not reading bugmail)]

In the case of:

   foo,
   {}

...IMHO the error _is_ on the second line. There's nothing wrong with the first
line. The error is that the second line had no selector.

Comment 5

[Boris Zbarsky [:bzbarsky]]

Oh, I see.  The testcase for this bug is bogus; fixing it.  Ian, I was talking
about the following testcase:

foo, {
}

Here we catch the error after reading the first '{', at which point the scanner
has advanced to the next line (at least that's the only way to explain the line
number we end up with, which is 2).

Comment 6

[Hixie (not reading bugmail)]

Oh, ok. Yeah, makes sense for that number to be earlier, but it's not worth
slowing anything down or increasing memory usage or anything...