Bug 32072 - Clean up invalid @-rule error handling in CSS
Summary: Clean up invalid @-rule error handling in CSS
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: CSS (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Dave Hyatt
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-12-02 09:40 PST by Dave Hyatt
Modified: 2009-12-15 00:11 PST (History)
2 users (show)

See Also:


Attachments
Patch (11.34 KB, patch)
2009-12-02 09:42 PST, Dave Hyatt
darin: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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