[webkit-reviews] review granted: [Bug 62424] [GTK] webkit_web_settings_copy does not copy all settings : [Attachment 96831] Patch

[bugzilla-daemon at webkit.org]

Xan Lopez <xan.lopez at gmail.com> has granted Martin Robinson
<mrobinson at webkit.org>'s request for review:
Bug 62424: [GTK] webkit_web_settings_copy does not copy all settings
https://bugs.webkit.org/show_bug.cgi?id=62424

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

------- Additional Comments from Xan Lopez <xan.lopez at gmail.com>
View in context: https://bugs.webkit.org/attachment.cgi?id=96831&action=review

OK with those changes, I guess, but I feel this will bite us down the road
somehow ;)

> Source/WebKit/gtk/tests/testwebsettings.c:52
> +    g_assert_cmpstr(defaultEncoding, ==, "utf-8");

Leaking defaultEncoding!

> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:1252
> +    for (size_t i = 0; i < numberOfProperties; i++) {

I believe to be at least a bit more correct you should check the property has a
G_PARAM_CONSTRUCT flag. And uh... READWRITE?

> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:1253
> +	   GParamSpec* property = properties.get()[i];

There is no nicer way to write this? at least properties[i].get() ?

> Source/WebKit/gtk/webkit/webkitwebsettings.cpp:1254
> +	   GParameter* parameter = parameters.get() + i;

And why do you do it differently in the next line?