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

Wed Oct 12 09:46:49 PDT 2011

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

Martin Robinson <mrobinson at webkit.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #110648|review?, commit-queue?      |review-
               Flag|                            |

--- Comment #46 from Martin Robinson <mrobinson at webkit.org>  2011-10-12 09:46:49 PST ---
(From update of attachment 110648)
View in context: https://bugs.webkit.org/attachment.cgi?id=110648&action=review

Okay! This is so close!

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:46
> +    _WebKitSettingsPrivate()
> +    {
> +        preferences = adoptWK(WKPreferencesCreate());
> +    }

Do you mind removing this contructor and just moving this line to webkit_settings_init? This is a neat approach, but we sould be doing it the same way in all classes.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:50
> + * SECTION:webkitwebsettings

This heading is now wrong. It should be webkitsettings.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:60
> + * g_object_set (G_OBJECT(settings), "enable-java-script", FALSE, NULL);

You're missing a space after G_OBJECT. This should be enable-java-script.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:74
> +    PROP_LOADS_ICONS_IGNORING_IMAGE_LOAD_PREFERENCE,

Do you mind changing this setting to PROP_LOAD_ICONS_IGNORING_IMAGE_LOAD_SETTING. Note that LOADS is now LOAD. I changed PREFERENCE to setting here because in the C API these are "preferences," but in our API they are "settings."

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:239
> +    * WebKitSettings:loads-icons-ignoring-image-load-preference
> +    *
> +    * Determines whether a site can load favicons irrespective
> +    * of the value of #WebSettings:auto-load-images.
> +    *
> +    */
> +    g_object_class_install_property(gObjectClass,
> +                                    PROP_LOADS_ICONS_IGNORING_IMAGE_LOAD_PREFERENCE,
> +                                    g_param_spec_boolean("loads-icons-ignoring-image-load-preference",
> +                                                         _("Loads site icons ignoring image load preference"),
> +                                                         _("Whether to load site icon ignoring image load preference."),
> +                                                         FALSE,
> +                                                         readWriteConstructParamFlags));
> +

Please change this setting to load-icons-ignoring-image-load-setting. The string "Whether to load site icon ignoring image load preference." should be "Whether to load site icons ignoring image load setting." Note that icons is plural now.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:244
> +    * web application cache ensures web applications are available even when

ensure to "allow" maybe. Ensure suggests that it works with all web applications automatically.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:266
> +    * http://dev.w3.org/TR/webstorage/.

This link doesn't work! Looks like the real link is http://dev.w3.org/html5/webstorage/

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:317
> +    * On touch devices, scrollable subframes on a page can result in a confusing user experience.

No comma necessary after "On touch devices"

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:376
> +    * Hyperlink auditing specification is available at

"The hyperlink auditing"...

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:414
> + * Get the property #WebKitSettings:enable-javascript.

Should be "Get the #WebKitSettings:enable-javascript property." Please make this fix for all properties below as well.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:419
> + * @see_also: webkit_settings_set_enable_javascript()

I think it's more useful to link to the property here since that's where the description of the feature is. Since you've already linked above, you can probably remove this line if you like. Please make this fix for all properties below as well.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:454
> + * Get the property #WebKitSettings:auto-load-images.

Ditto.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:458
> + * @see_also: webkit_settings_set_auto_load_images()

Ditto.

> Source/WebKit2/UIProcess/API/gtk/WebKitSettings.cpp:504
> + * webkit_settings_get_loads_icons_ignoring_image_load_preference:
> + * @settings: a #WebKitSettings
> + *
> + * Get the property #WebKitSettings:loads-icons-ignoring-image-load-preference.
> + *
> + * Returns: %TRUE If site icon can be loaded irrespective of image loading preference or %FALSE otherwise.
> + *
> + * @see_also: webkit_settings_set_loads_icons_ignoring_image_load_preference()
> + **/
> +gboolean webkit_settings_get_loads_icons_ignoring_image_load_preference(WebKitSettings* settings)
> +{
> +    g_return_val_if_fail(WEBKIT_IS_WEB_SETTINGS(settings), FALSE);
> +
> +    return WKPreferencesGetLoadsSiteIconsIgnoringImageLoadingPreference(settings->priv->preferences.get());
> +}

Don't forget to update preference -> setting here.

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