Bug 32072

Summary: Clean up invalid @-rule error handling in CSS
Product: WebKit Reporter: Dave Hyatt <hyatt>
Component: CSSAssignee: Dave Hyatt <hyatt>
Status: RESOLVED FIXED    
Severity: Normal CC: hamaji, webkit.review.bot
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch darin: review+

Description Dave Hyatt 2009-12-02 09:40:34 PST
Lots of mistakes, mostly because of the grammar trying to enforce the ordering of namespace vs. import vs. normal rules.
Comment 1 Dave Hyatt 2009-12-02 09:42:46 PST
Created attachment 44158 [details]
Patch
Comment 2 WebKit Review Bot 2009-12-02 09:43:26 PST
style-queue ran check-webkit-style on attachment 44158 [details] without any errors.
Comment 3 Darin Adler 2009-12-02 09:47:30 PST
Comment on attachment 44158 [details]
Patch

> +    , m_allowImportRules(true)
> +    , m_allowVariablesRules(true)
> +    , m_allowNamespaceDeclarations(true)

Normally I'd prefer not to name data members with verb phrases. That's why we end up with names like "shouldAllow". Not sure I have better suggestions here, though.

> +    m_allowImportRules = m_allowNamespaceDeclarations = m_allowVariablesRules = false;

I'd prefer to see three conventional assignment statements instead of this one-liner.

I'm not sure all the new error conditions are being logged for the inspector's benefit.

The test case doesn't seem to cover everything you changed.

Despite the minor concerns above, r=me
Comment 4 Dave Hyatt 2009-12-02 11:27:47 PST
Landed in r51608.
Comment 5 Shinichiro Hamaji 2009-12-15 00:11:28 PST
It seems the background color of fast/css/variables/misplaced-import-test becomes red by this patch. Was this intentional? If so, we may need to fix the test.

http://trac.webkit.org/changeset/51608#file6