I Steps: Go to the attached reduction II Issue: The div is not showing blue background III Conclusion: comment and the @charset ".." causes the rest of the css to be ignored. IV Other browsers: IE7: ok FF3: ok Opera9.24: ok V Nightly tested: 31446
Created attachment 20278 [details] reduction
Confirmed with r34277. The stylesheet is of course invalid, but that doesn't mean that we should have a different behavior than other browsers.
Created attachment 22917 [details] further reduction
Variant of the bug is covered by one test http://samples.msdn.microsoft.com/csstestpages/Chapter_4/Rules/at-rule-005.htm of the Microsoft CSS 2.1 Test Suite http://samples.msdn.microsoft.com/csstestpages/
See also: bug 23744 (misplaced @comment in external CSS).
*** Bug 28888 has been marked as a duplicate of this bug. ***
*** Bug 28821 has been marked as a duplicate of this bug. ***
Assigned for investigation.
*** Bug 23744 has been marked as a duplicate of this bug. ***
Created attachment 41305 [details] Patch v1
Please review this patch carefully. I don't actually have any idea what I'm doing.
Created attachment 41306 [details] Patch v1
Comment on attachment 41306 [details] Patch v1 Just a few nitpicks: > + @charset rule causes the rest of inline css to be ignored You've marked a bug talking about non-inline case as duplicate - could you please update the title, and add tests for external CSS case? Is any whitespace allowed by other browsers, or just a single linefeed after <style>? If it's the latter, perhaps HTML parser should consume the linefeed instead. > +++ b/LayoutTests/fast/css/comment-before-charset-expected.txt How did render tree dumps end up outside platform/mac? They should come with pixel results - but even better, these tests should be text-only (you can use getComputedStyle, or check document.styleSheets). Looks good to me otherwise.
(In reply to comment #13) > You've marked a bug talking about non-inline case as duplicate - could you > please update the title, and add tests for external CSS case? Done and will do. > Is any whitespace allowed by other browsers, or just a single linefeed after > <style>? If it's the latter, perhaps HTML parser should consume the linefeed > instead. I'll add more tests for these cases and verify that their results match other browsers. > > +++ b/LayoutTests/fast/css/comment-before-charset-expected.txt > > How did render tree dumps end up outside platform/mac? I moved them. I've never written a non-text test before, so I probably screwed that up. > They should come with > pixel results - but even better, these tests should be text-only (you can use > getComputedStyle, or check document.styleSheets). Will do.
Created attachment 41341 [details] Patch v1
Created attachment 41342 [details] Patch v1
Comment on attachment 41342 [details] Patch v1 Sorry, I now re-read the spec, and it says that we shouldn't allow the whitespace - we should ignore any @charset rule that isn't at the very beginning (except for possibly a BOM). "User agents must ignore any @charset rule not at the beginning of the style sheet." The difference is that an incorrect charset specified in a @charset rule will be ignored, and a default will be used (in most cases, the default will be main page encoding). I do not know if this ever happens in practice, but we did see examples of pages where a charset incorrectly specified in HTML head needed to be ignored. r=me, because this patch is clearly an improvement. If you're not too tired of this, you can consider matching the spec though.
Comment on attachment 41342 [details] Patch v1 I'm inclined to commit this change and file another bug for ignoring the charset because that change seems like a lower priority than fixing this issue.
Comment on attachment 41342 [details] Patch v1 Builders are red and I need to head home for the day. I'll land this tonight.
Comment on attachment 41342 [details] Patch v1 Clearing flags on attachment: 41342 Committed r49727: <http://trac.webkit.org/changeset/49727>
All reviewed patches have been landed. Closing bug.
*** Bug 29046 has been marked as a duplicate of this bug. ***