Bug 12145

Summary: Color parsing is too relaxed in strict mode
Product: WebKit Reporter: Sam Weinig <sam>
Component: CSSAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal    
Priority: P2    
Version: 420+   
Hardware: Mac   
OS: OS X 10.4   
URL: http://www.bjoernsworld.de/test/color.html
Attachments:
Description Flags
testcase (strict mode)
none
testcase (quirks mode)
none
First attempt darin: review+

Sam Weinig
Reported 2007-01-06 16:36:54 PST
The testcase pointed to in the URL field demonstrates a way in which the css color parsing is too relaxed. In particular, hexidecimal color values without a leading # should be illegal. This test passes in Firefox 2, IE7 and Opera 9, but not in either shipping Safari or WebKit ToT.
Attachments
testcase (strict mode) (343 bytes, text/html)
2007-01-06 18:22 PST, Sam Weinig
no flags
testcase (quirks mode) (246 bytes, text/html)
2007-01-06 18:23 PST, Sam Weinig
no flags
First attempt (28.05 KB, patch)
2007-01-09 14:02 PST, Rob Buis
darin: review+
Dave Hyatt
Comment 1 2007-01-06 17:33:11 PST
Try the same test without the doctype (in quirks mode). Probably a quirk that isn't properly qualified.
Sam Weinig
Comment 2 2007-01-06 18:21:14 PST
(In reply to comment #1) > Try the same test without the doctype (in quirks mode). Probably a quirk that > isn't properly qualified. You are correct. In quirks mode Firefox, IE and Opera, the line does get successfully parsed.
Sam Weinig
Comment 3 2007-01-06 18:22:37 PST
Created attachment 12271 [details] testcase (strict mode)
Sam Weinig
Comment 4 2007-01-06 18:23:18 PST
Created attachment 12272 [details] testcase (quirks mode)
Rob Buis
Comment 5 2007-01-09 14:02:39 PST
Created attachment 12336 [details] First attempt I figured out the Format substitution (http://trac.webkit.org/projects/webkit/changeset/17700) broke color parsing like color: 33FFFF. Related is handling of FF33333, as shown by the testcases for this bug. This patch should fix both. Cheers, Rob.
Darin Adler
Comment 6 2007-01-09 14:15:32 PST
Comment on attachment 12336 [details] First attempt Looks fine, r=me. + if (Color::parseHexColor(name, rgb) && !strict) return true; It would be nice to not even parse in the strict case. Just reverse the two halves of the &&. + static RGBA32 parseColor(const String&, bool = false); Should have the name strict here, since bool along doesn't make it clear what the parameter is. + static bool parseColor(const String&, RGBA32& rgb, bool); Ditto.
Rob Buis
Comment 7 2007-01-09 14:47:21 PST
Landed in r18722.
Note You need to log in before you can comment on or make changes to this bug.