Closed
Bug 810651
Opened 13 years ago
Closed 13 years ago
Missing “]” in selector causes rest of style sheet to be ignored
Categories
(Firefox :: Untriaged, defect)
Tracking
()
RESOLVED
INVALID
People
(Reporter: jukka.k.korpela, Unassigned)
Details
User Agent: Mozilla/5.0 (Windows NT 6.1; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121024073032
Steps to reproduce:
An attribute selector lacks a closing “]”. Simple complete HTML document example:
<!doctype html>
<style>
input[type="text" {
background: yellow;
}
body {
background: green;
}
</style>
Hello world
<input>
Actual results:
The rest of the style sheet gets ignored. This can be seen using Firebug to inspect the situation. In the simple example, the body has default background (white), not the declared green background.
Expected results:
Only the statement containing the syntax error should be ignored. In the example case, the body background should be green.
CSS 2.1 error handling rules, clause 4.2, item “Malformed statements” says: “User agents must handle unexpected tokens encountered while parsing a statement by reading until the end of the statement, while observing the rules for matching pairs of (), [], {}, "", and '', and correctly handling escapes. For example, a malformed statement may contain an unexpected closing brace or at-keyword.” So the parser should skip the rest of the statement, not the rest of the stylesheet.
It might be argued that the reference to “rules for matching pairs” means that a parser should or could keep looking for the matching “]” down to the end of the style sheet. But this is not what the W3C CSS Validator does, or what other browsers do. They stop skipping at the “}” that ends the statement. Besides, throwing in “]” between statements does not stop Firefox from skipping to the end of the style sheet.
|
||
Comment 1•13 years ago
|
||
This is invalid. The "rules for matching pairs" does in fact mean exactly that you have to find a ']' that's not inside any other constructs.
> this is not what the W3C CSS Validator does
It has a pretty buggy parser.
> or what other browsers do
Which other browsers? WebKit's parser doesn't implement CSS error recovery correctly. Opera does what we do.
Status: UNCONFIRMED → RESOLVED
Closed: 13 years ago
Resolution: --- → INVALID
|
|
||
Comment 2•13 years ago
|
||
And to be precise, the recovery rules say that the UA needs to fine the nearest ']' that's not inside any other constructs and then read until end of statement from that point (while continuing to match pairs, etc). So if you do this:
input[type="text" {
background: yellow;
}]
body {
background: green;
}
body { color: purple; }
You will get purple text on default background, because the end of statement that's found is the '}' of the first "body" rule.
|
|
Reporter | |
Comment 3•13 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #2)
> So if you do this:
[...]
> You will get purple text on default background, because the end of statement
> that's found is the '}' of the first "body" rule.
I don’t; on Firefox 16.0.2, I get default color on default background; the rest of the style sheet is still ignored, despite the added “]” (which I mentioned in my original report).
So this looks very much like error handling gone wild rather than planned behavior based on an interpretation of error handling rules that differs from the interpretation of the CSS Validator and Chrome.
|
|
||
Comment 4•13 years ago
|
||
> I don’t; on Firefox 16.0.2, I get default color on default background;
Can you attach your testcase, please? The entire file you're using? When I use the rules from comment 2, I get purple text in Firefox 16, as expected.
> So this looks very much like error handling gone wild rather than planned behavior
Having implemented parts of this code, I can assure you that it is quite planned behavior.
It's a well-documented fact that WebKit has a very poor implementation of CSS 2.1 section 4.2. I suggest looking at all the red in the relevant part of http://www.w3.org/Style/CSS/Test/CSS2.1/20110323/reports/results.html
In particular, there's a test for the behavior we're discussing in the official CSS 2.1 test suite (http://test.csswg.org/suites/css2.1/20110323/html4/matching-brackets-003.htm ), and Chrome quite by coincidence fails that test.
|
|
Reporter | |
Comment 5•13 years ago
|
||
My test for the effect of adding “]” later was
<!doctype html>
<style>
input[type="text" {
background: yellow;
}
]
body {
background: green;
}
</style>
Hello world
<input>
I.e., just “]” inserted between rules. I now realize this wasn’t sufficient, since if the “]” is parsed as terminating the attribute selector, then the end of statement will only be after the next “}”.
I still think that your interpretation of CSS 2.1 is not the only feasible option, or even the most natural one. It is certainly impractical, since it means that the forgotten “]” almost always causes the rest of the style sheet to be ignored, since there usually isn’t any unmatched “]” later on (that is, a “]” that does not match any “[” in the user’s intentions).
(I wonder what makes the CSS test suite more official than the W3C CSS Validator.)
|
|
||
Comment 6•13 years ago
|
||
> It is certainly impractical
It's the only one that makes parsing actually forward-compatible. Consider a future CSS spec that introduces a selector like so:
[attr={regexp}]
that would do a regexp match on the value. What would your proposed parsing do with that in UAs that don't yet support the new selector?
Keep in mind the point of the error recovery rules in CSS is not to make syntactically invalid stylesheets "work" in general. The point is to make it possible to introduce new constructs to CSS while not completely breaking UAs that don't implement the new construct yet.
> (I wonder what makes the CSS test suite more official than the W3C CSS Validator.)
The fact that the CSS working group has reviewed the test suite but has no control over the CSS validator and that bug reports against the validator have a tendency to be ignored.
You need to log in
before you can comment on or make changes to this bug.
Description
•