Bug 71111 - [GTK] Add webkit_settings_new_with_settings() to WebKit2 GTK+ API
Summary: [GTK] Add webkit_settings_new_with_settings() to WebKit2 GTK+ API
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit2 (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P2 Normal
Assignee: Nobody
URL:
Keywords: Gtk
Depends on:
Blocks:
 
Reported: 2011-10-28 05:11 PDT by Carlos Garcia Campos
Modified: 2011-11-02 07:37 PDT (History)
6 users (show)

See Also:


Attachments
Patch (12.02 KB, patch)
2011-10-28 05:15 PDT, Carlos Garcia Campos
mrobinson: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Carlos Garcia Campos 2011-10-28 05:11:51 PDT
It's a convenient method to create a WebKitSettings object. Probably one of the first things someone would do after creating a WebKitSettings object is set some preferences, because otherwise it wouldn't get the settings from the view with the default values instead of creating a new one. So with this method you could do:

settings = webkit_settings_new_with_settings("enable-javascript", FALSE,
                                                                             "auto-load-images", FALSE,
                                                                             "load-icons-ignoring-image-load-setting", TRUE,
                                                                              NULL);
Comment 1 Carlos Garcia Campos 2011-10-28 05:15:02 PDT
Created attachment 112850 [details]
Patch
Comment 2 WebKit Review Bot 2011-10-28 05:17:00 PDT
Attachment 112850 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebKit2/ChangeLog', u'Source/WebKit..." exit_code: 1

Source/WebKit2/UIProcess/API/gtk/tests/TestWebKitSettings.cpp:114:  Use 0 instead of NULL.  [readability/null] [5]
WARNING: File exempt from style guide. Skipping: "Source/WebKit2/UIProcess/API/gtk/WebKitSettings.h"
Total errors found: 1 in 5 files


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 3 WebKit Review Bot 2011-10-28 05:17:15 PDT
Thanks for the patch. If this patch contains new public API please make sure it follows the guidelines for new WebKit2 GTK+ API. See http://trac.webkit.org/wiki/WebKitGTK/AddingNewWebKit2API
Comment 4 Martin Robinson 2011-10-28 06:47:12 PDT
Comment on attachment 112850 [details]
Patch

Neat-o.
Comment 5 Carlos Garcia Campos 2011-10-31 04:56:10 PDT
Committed r98846: <http://trac.webkit.org/changeset/98846>
Comment 6 Gustavo Noronha (kov) 2011-11-01 04:48:17 PDT
Do you think this is really useful? It seems to me like something that could easily be achieved simply by using g_object_new() without any loss of convenience.
Comment 7 Martin Robinson 2011-11-01 06:48:52 PDT
(In reply to comment #6)
> Do you think this is really useful? It seems to me like something that could easily be achieved simply by using g_object_new() without any loss of convenience.

Gustavo makes a good point here. It does seem to overlap a bit with g_object_new().
Comment 8 Carlos Garcia Campos 2011-11-02 00:45:01 PDT
Every foo_new() method overlaps g_object_new(), in most the cases, calling g_object_new() is the only thing it does. With this simple api is pretty obvious that you can create a settings object with some initial settings, because the function name says that and because it's documented. Ephy, midori and yelp do webkit_web_setting_new() + g_object_set(), and I bet all other apps using webkitgtk do the same.
Comment 9 Gustavo Noronha (kov) 2011-11-02 07:15:24 PDT
That's true, the no-arguments new() functions are estabilished convention, and there's a case to be made that they make it more convenient. Anyway, I don't feel strongly about this, if you think it'll be convenient I'm ok with it, but maybe we should look at renaming it.

The name sounded weird to me the first time I read, it was not immediately obvious that it would take property name:value pairs. All new_with_something() functions I could find in gobject libraries take a fixed number of arguments, all vararg functions seem to be named new_full(), what do you think of following that convention?

(hrm. I found an exception: gtk_tree_view_column_new_with_attributes(), maybe new_with_values() would make it more obvious too?)
Comment 10 Carlos Garcia Campos 2011-11-02 07:37:45 PDT
Good point, I prefer new_with_values(), new_full() is still confusing, I think.