Bug 14346 – CSS parser accepts unexpected { } in declarations

• Status: VERIFIED
• Resolution: FIXED
• Component: CSS

[Gérard Talbot (no longer involved)]

<style type="text/css">
p { color:green; color{;color:maroon} } 
</style>

<p>This text should be green</p>

MSIE 7, Opera 9.21, Firefox 2.0.0.4 all pass this test.
This test directly comes from a given example in
CSS 2.1, section 4.2 Rules for handling parsing errors, malformed declarations item:
http://www.w3.org/TR/CSS21/syndata.html#parsing-errors

[Gérard Talbot (no longer involved)]

Safari 3.0.2 build 522.13.1 here.

[Gérard Talbot (no longer involved)]

Hello,

Please confirm this bug report: the testcase is clear, clean, reduced and comes directly from the latest CSS 2.1 WD in a given example. Thank you.

[Alexey Proskuryakov]

Confirmed with r24287. But please note that fixing this maybe inappropriate during stabilization period anyway, if this problem doesn't affect any sites.

[Anatoli Papirovski]

Created attachment 20925 [details]
patch

(One note to begin with, there's hardly any logic to the formatting of the grammar file, so I just tried to match whatever was there... )

The changelog has most of the fixes listed in it. I'll just provide the test cases not in our test suite that were fixed by this patch:
http://hixie.ch/tests/adhoc/css/parsing/core-syntax/002-demo.html
http://hixie.ch/tests/adhoc/css/parsing/core-syntax/002.html
http://hixie.ch/tests/adhoc/css/parsing/core-syntax/003.html
http://hixie.ch/tests/adhoc/css/parsing/core-syntax/006.html
http://dbaron.org/css/test/parsing4
http://dbaron.org/css/test/parsing5
http://samples.msdn.microsoft.com/csstestpages/Chapter_4/eof-001.htm
http://samples.msdn.microsoft.com/csstestpages/Chapter_4/invalid-decl-at-rule-001.htm (wrong description inside, but a correct test case)
http://annevankesteren.nl/test/css/syntax/007.html
http://annevankesteren.nl/test/css/syntax/018.html

+ the three test cases from the CSS 2.1 test suite which are in LayoutTests.

[Dave Hyatt]

Comment on attachment 20925 [details]
patch

r=me

[Robert Blaut]

*** Bug 19567 has been marked as a duplicate of this bug. ***

[Robert Blaut]

*** Bug 20102 has been marked as a duplicate of this bug. ***

[Jacob Refstrup]

Created attachment 22380 [details]
Patch -- fixed against r35223

Fixed merge conflict against r35223; tested this patch against test case for 15423 and this patch also fixes that defect so I'll close set 15423 as a duplicate.

[Jacob Refstrup]

(In reply to comment #8)
> Created an attachment (id=22380) [edit]
> Patch -- fixed against r35223
> 
> Fixed merge conflict against r35223; tested this patch against test case for
> 15423 and this patch also fixes that defect so I'll close set 15423 as a
> duplicate.
> 

Already marked as a dependent.

[David Kilzer (:ddkilzer)]

(In reply to comment #8)
> Created an attachment (id=22380) [edit]
> Patch -- fixed against r35223
> 
> Fixed merge conflict against r35223; tested this patch against test case for
> 15423 and this patch also fixes that defect so I'll close set 15423 as a
> duplicate.

Merged patch fails building with r35256:

bison -d -p cssyy WebCore/css/CSSGrammar.y -o CSSGrammar.cpp
WebCore/css/CSSGrammar.y:336: type clash (`' `rule') on default action
make: *** [CSSGrammar.cpp] Error 1

[David Kilzer (:ddkilzer)]

(In reply to comment #10)
> Merged patch fails building with r35256:
> 
> bison -d -p cssyy WebCore/css/CSSGrammar.y -o CSSGrammar.cpp
> WebCore/css/CSSGrammar.y:336: type clash (`' `rule') on default action
> make: *** [CSSGrammar.cpp] Error 1

I have a fix for this that reduces the increased shift/reduce count by one (from 3 to 2) for the patch.

[David Kilzer (:ddkilzer)]

Committed revision 35261.

http://trac.webkit.org/changeset/35261

Note that I had to change the patch to CSSGrammar.y to work with trunk:

- Reduced new shift/reduce conflicts by 1 (from 3 to 2)
- Removed "%type <rule> invalid_at_list"
- Moved "%type <ruleList> block_rule_list" to where "%type <ruleList> rule_list" used to be [cosmetic]
- Changed "invalid_at_list" definition to match other *_list definitions (like namespace_list)

[Robert Blaut]

During building Webkit I got now:

bison -d -p cssyy WebCore/css/CSSGrammar.y -o CSSGrammar.cpp
WebCore/css/CSSGrammar.y: conflicts: 46 shift/reduce, 17 reduce/reduce
WebCore/css/CSSGrammar.y: expected 0 reduce/reduce conflicts
WebCore/css/CSSGrammar.y:1319.16: warning: rule never reduced because of conflicts: invalid_at_list: /* empty */
make: *** [CSSGrammar.cpp] Error 1
** BUILD FAILED **

[Gavin Barraclough]

Patch reverted due to build issues (in CSSGrammar.y, as per previous comment).

Sending        LayoutTests/ChangeLog
Sending        LayoutTests/platform/mac/css2.1/t040105-import-01-b-expected.checksum
Deleting       LayoutTests/platform/mac/css2.1/t040105-import-01-b-expected.png
Sending        LayoutTests/platform/mac/css2.1/t040105-import-01-b-expected.txt
Sending        LayoutTests/platform/mac/css2.1/t0402-syntax-05-f-expected.checksum
Deleting       LayoutTests/platform/mac/css2.1/t0402-syntax-05-f-expected.png
Sending        LayoutTests/platform/mac/css2.1/t0402-syntax-05-f-expected.txt
Sending        LayoutTests/platform/mac/css2.1/t0402-syntax-06-f-expected.checksum
Deleting       LayoutTests/platform/mac/css2.1/t0402-syntax-06-f-expected.png
Sending        LayoutTests/platform/mac/css2.1/t0402-syntax-06-f-expected.txt
Sending        WebCore/ChangeLog
Sending        WebCore/css/CSSGrammar.y
Transmitting file data .........
Committed revision 35266.

[Gavin Barraclough]

Reopening bug.

[David Kilzer (:ddkilzer)]

(In reply to comment #13)
> During building Webkit I got now:
> 
> bison -d -p cssyy WebCore/css/CSSGrammar.y -o CSSGrammar.cpp
> WebCore/css/CSSGrammar.y: conflicts: 46 shift/reduce, 17 reduce/reduce
> WebCore/css/CSSGrammar.y: expected 0 reduce/reduce conflicts
> WebCore/css/CSSGrammar.y:1319.16: warning: rule never reduced because of
> conflicts: invalid_at_list: /* empty */
> make: *** [CSSGrammar.cpp] Error 1
> ** BUILD FAILED **

Sorry, I guess Tiger's version of bison isn't as stringent as Leopard's.

[David Kilzer (:ddkilzer)]

(In reply to comment #16)
> (In reply to comment #13)
> > bison -d -p cssyy WebCore/css/CSSGrammar.y -o CSSGrammar.cpp
> > WebCore/css/CSSGrammar.y: conflicts: 46 shift/reduce, 17 reduce/reduce
> > WebCore/css/CSSGrammar.y: expected 0 reduce/reduce conflicts
> > WebCore/css/CSSGrammar.y:1319.16: warning: rule never reduced because of
> > conflicts: invalid_at_list: /* empty */
> > make: *** [CSSGrammar.cpp] Error 1
> > ** BUILD FAILED **
> 
> Sorry, I guess Tiger's version of bison isn't as stringent as Leopard's.

I have a fix for this on Tiger now.  Need to test the fix on Leopard as well.

[David Kilzer (:ddkilzer)]

$ git svn dcommit
Committing to http://svn.webkit.org/repository/webkit/trunk ...
	M	LayoutTests/ChangeLog
	M	LayoutTests/platform/mac/css2.1/t040105-import-01-b-expected.checksum
	A	LayoutTests/platform/mac/css2.1/t040105-import-01-b-expected.png
	M	LayoutTests/platform/mac/css2.1/t040105-import-01-b-expected.txt
	M	LayoutTests/platform/mac/css2.1/t0402-syntax-05-f-expected.checksum
	A	LayoutTests/platform/mac/css2.1/t0402-syntax-05-f-expected.png
	M	LayoutTests/platform/mac/css2.1/t0402-syntax-05-f-expected.txt
	M	LayoutTests/platform/mac/css2.1/t0402-syntax-06-f-expected.checksum
	A	LayoutTests/platform/mac/css2.1/t0402-syntax-06-f-expected.png
	M	LayoutTests/platform/mac/css2.1/t0402-syntax-06-f-expected.txt
	M	WebCore/ChangeLog
	M	WebCore/css/CSSGrammar.y
Committed r35403