[webkit-reviews] review granted: [Bug 32072] Clean up invalid @-rule error handling in CSS : [Attachment 44158] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Dave Hyatt <hyatt at apple.com>'s
request for review:
Bug 32072: Clean up invalid @-rule error handling in CSS
https://bugs.webkit.org/show_bug.cgi?id=32072

Attachment 44158: Patch
https://bugs.webkit.org/attachment.cgi?id=44158&action=review

------- Additional Comments from Darin Adler <darin at apple.com>
> +    , 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