[webkit-reviews] review denied: [Bug 90298] Add new CSS property "-webkit-widget-region" to expose dashboard region support for other port : [Attachment 150230] Proposed Patch

[bugzilla-daemon at webkit.org]

Dmitry Titov <dimich at chromium.org> has denied Jian Li <jianli at chromium.org>'s
request for review:
Bug 90298: Add new CSS property "-webkit-widget-region" to expose dashboard
region support for other port
https://bugs.webkit.org/show_bug.cgi?id=90298

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

------- Additional Comments from Dmitry Titov <dimich at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=150230&action=review


I quickly looked at the patch itself... 
It seems it doesn't build on GTK (see EWS failure).
Also, it adds a CSS property but does not have tests that verify that it is
actually exposed. Could you add a test or is the plan to not enable the feature
now and enable it in builds with a test as a subsequent patch? 

It seems ok to use the existing support that was implemented for Dashboard
Widgets (if not, I hope Apple folks will chime in) - but it seems, on a cursory
look, that test support for this is thin. If we are to use this mechanism,
we'll need to add way more layout tests. Can be done in other patches, of
course.

> Source/WebCore/css/CSSParser.cpp:4252
> +#if ENABLE(DASHBOARD_SUPPORT)

This makes DASHBOARD_SUPPORT and WIDGET_REGION mutually exclusive, when they
should not be, or we can't have a build with both enabled...