Summary: | Clean up invalid @-rule error handling in CSS | ||||||
---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Dave Hyatt <hyatt> | ||||
Component: | CSS | Assignee: | 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
Dave Hyatt
2009-12-02 09:40:34 PST
Created attachment 44158 [details]
Patch
style-queue ran check-webkit-style on attachment 44158 [details] without any errors.
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 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 |