[webkit-reviews] review denied: [Bug 116043] [GTK] Provide web setting to control CSS regions run time : [Attachment 201581] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 13 11:00:22 PDT 2013


Carlos Garcia Campos <cgarcia at igalia.com> has denied Anton Obzhirov
<a.obzhirov at samsung.com>'s request for review:
Bug 116043: [GTK] Provide web setting to control CSS regions run time
https://bugs.webkit.org/show_bug.cgi?id=116043

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

------- Additional Comments from Carlos Garcia Campos <cgarcia at igalia.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=201581&action=review


Thanks for the patch. I'm not sure if we want to add new API to WebKit1 which
is currently in maintenance mode. I would split the patch into one for wk1 and
one for wk2 in any case so that it can be discussed separately.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:34
> +#include "RuntimeEnabledFeatures.h"

Please use  angle-bracket form for WebCore header file includes.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1113
> +	* Since: 2.0

This should be 2.2, 2.0 was already released.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:1120
> +	       TRUE,

I don't think we want this to be enabled by default, at least not for now.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2753
> + */

Since 2.2.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2767
> + */

Since 2.2.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:2779
> +#if ENABLE(CSS_REGIONS)
> +    WebCore::RuntimeEnabledFeatures::setCSSRegionsEnabled(enabled);
> +#endif

Do we really need to do this in the UI process too?


More information about the webkit-reviews mailing list