[webkit-reviews] review denied: [Bug 102344] Implement CSSHostRule for @host @-rules. : [Attachment 179393] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Dec 13 20:45:16 PST 2012


Dimitri Glazkov (Google) <dglazkov at chromium.org> has denied Takashi Sakamoto
<tasak at google.com>'s request for review:
Bug 102344: Implement CSSHostRule for @host @-rules.
https://bugs.webkit.org/show_bug.cgi?id=102344

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

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


> Source/WebCore/ChangeLog:15
> +	   * CMakeLists.txt:

Would love to have a bit more detail on these.

> Source/WebCore/css/CSSHostRule.cpp:24
> +#include "CSSHostRule.h"

This whole file seems like a lot of boilerplate that's probably very similar to
@region stuff and other similar at-rules handling. I sense a great code health
improvement opportunity! :)

> Source/WebCore/css/StyleRule.h:251
> +class StyleRareRuleBlock : public StyleRuleBlock {

The use of a Rare pattern here seems like a bit of an overkill. We will only
have these temporarily and probably won't have many at once. Tab and I should
have @host become 17 shortly.

Can we just have the id be 17 internally and 1001 externally?


More information about the webkit-reviews mailing list