[Webkit-unassigned] [Bug 68371] [GTK][WEBKIT2] Add WebKitWebSettings GTK+ API

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=68371





--- Comment #19 from Nayan Kumar K <nayankk at motorola.com>  2011-09-22 07:00:37 PST ---
(From update of attachment 108286)
View in context: https://bugs.webkit.org/attachment.cgi?id=108286&action=review

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:83
>> +static void webkitWebSettingsSetProperty(GObject* object, guint prop_id, const GValue* value, GParamSpec* pspec)
> 
> It should be propId instead of prop_id

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:89
>> +        webkit_web_settings_set_java_script_enabled(settings, (bool)g_value_get_boolean(value));
> 
> webkit_web_settings_set_java_script_enabled() takes a gboolean, bool cast is wrong there.

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:92
>> +        webkit_web_settings_set_loads_images_automatically(settings, (bool)g_value_get_boolean(value));
> 
> I think methods should follow the pattern webkit_web_settings_[get|set]_property_name(), in this case webkit_web_settings_set_auto_load_images()

Done. I followed the naming convention exposed by WKPreference. But, yes, it does make sense to follow webkit_web_settings_[get|set]_property_name convention

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:229
>> +                                                         "load-icon-ignoring-image-load-pref",
> 
> The name is long, but I think we should still use preference instead of pref

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:248
>> +                                                         FALSE,
> 
> This is enabled by default in WebKit1, I think we should use the same default values to make the transition easier and consistent.

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:360
>> +                                                         TRUE,
> 
> This is FALSE by default in WebKit1

Done.

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:374
>> +                                                         TRUE,
> 
> This is FALSE by default in WebKit1

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:406
>> + * Returns: TRUE - If Java Script is enabled.
> 
> Use %TRUE and remove the -

Done

>> Source/WebKit2/UIProcess/API/gtk/WebKitWebSettings.cpp:412
>> +    g_return_val_if_fail(WEBKIT_IS_WEB_SETTINGS(settings), false);
> 
> The function returns a gboolean, use FALSE instead of false here.

Done

>> Source/WebKit2/UIProcess/API/gtk/tests/testwebcontext.c:27
>> +    g_assert(webkit_web_context_get_default() == webkit_web_context_get_default());
> 
> This change is unrelated. I already fixed it in bug https://bugs.webkit.org/show_bug.cgi?id=67931.

Yes, I realized it after submitting the patch and added new one later excluding this change

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.