[webkit-reviews] review granted: [Bug 89542] Add runtime flag to enable/disable CSS variables (in addition to existing compile-time flag). : [Attachment 148951] Patch

[bugzilla-daemon at webkit.org]

Dimitri Glazkov (Google) <dglazkov at chromium.org> has granted Luke Macpherson
<macpherson at chromium.org>'s request for review:
Bug 89542: Add runtime flag to enable/disable CSS variables (in addition to
existing compile-time flag).
https://bugs.webkit.org/show_bug.cgi?id=89542

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

------- Additional Comments from Dimitri Glazkov (Google)
<dglazkov at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=148951&action=review


Looks good. Please take care of the comments before landing.

> Source/WebCore/ChangeLog:8
> +	   No new tests.

Actually, you _are_ making changes to tests, so perhaps a more accurate way to
say it would be "Tests adapted to use this functionality."

> Source/WebCore/ChangeLog:16
> +	   (WebCore::CSSParser::detectDashToken):
> +	   (WebCore::CSSParser::lex):

I don't see where you're making changes to these two functions in the patch.

> Source/WebCore/css/CSSParser.cpp:1027
> +bool CSSParser::parseValue(StylePropertySet* declaration, CSSPropertyID
propertyID, const String& string, bool important, Document* document)

I am not sure what this function is doing here in this patch.