Bug 14809 – [gtk] Implement settings as properties of WebKitGtkPage

• Component: WebKit API
• Status: RESOLVED INVALID

[Christian Dywan]

We don't currently have a way of retrieving or modifying settings (fonts, colors, etc.). The proper way would be implementing a property of WebKitGtkPage for every value, along with get and set functions in the style webkit_gtk_set_variable_font.

[Christian Dywan]

Created attachment 16749 [details]
An attempt at gobject properties

This is a first attempt at implementing WebCore:Settings as properties of WebKitPage. This is intentionally only six selected properties because I would like someone to look at it before repeating bad code very often.

I plan to implement every value of WebCore::Settings. I don't know wether there should be getters and setters for all of them, just a few for the most common values or none at all.

Comments are welcome.

[Christian Dywan]

Created attachment 16870 [details]
Many more properties, style cleanup

This revision of the patch implements many more properties. Further more the code was cleaned up style wise.

[Christian Dywan]

Created attachment 17308 [details]
wrong file

This changes the behavior of the script-prompt to return NULL when the dialog is cancelled. This matches other browsers, tested with Firefox and Opera on a number of websites.

Besides I changed a bit of the dialog code to avoid compiler warnings regarding uninitialized values.

At last I modifed the console message default to prevent linkification in terminals to include the line number in the link.

[Christian Dywan]

(In reply to comment #3)
> Created an attachment (id=17308) [edit]
> Correct script prompt return value and avoid warrings.
> [snip]

Nevermind my mistake, this patch doesn't belong to this bug.

[Xan Lopez]

(In reply to comment #2)
> Created an attachment (id=16870) [edit]
> Many more properties, style cleanup
> 
> This revision of the patch implements many more properties. Further more the
> code was cleaned up style wise.
> 

Some comments:

- PROP_0 is generally used instead of PROP_NULL.
- Unneeded blank lines in the switch constructs.
- The first parameter of the param_spec functions is the canonical name of the property, which has to follow some syntactic rules. The second is the "nickname", which is generally the canonical name with s/-/ / and properly capitalized. The third one is a human readable description explaining what this property is about.
- You use lots of magic numbers :|

Other than that, looks good to me.

[Salvatore De Paolis]

Created attachment 17668 [details]
g_object_prefs set and get

This is the Christian's patch adapted to new API and xan advices.
About comments, I wish they could be enhance by someone else.

[Salvatore De Paolis]

Created attachment 17679 [details]
webkit: set_property and get_property

[Mark Rowe (bdash)]

Comment on attachment 17679 [details]
webkit: set_property and get_property

I think PROP_IMAGES_LOAD_AUTOMATICALLY and "auto-images-load" should be PROP_AUTO_LOAD_IMAGES and "auto-load-images" respectively.  "auto-images-load" doesn't make sense as-is.

The default fonts you have specified, amongst other things, do not appear to match those used in webkit_web_view_init.  Is there a reason for this?

In webkit_web_view_class_init  there needs to be a blank line before the property PROP_FONT_SIZE_DEFAULT_FIXED is created to be consistent.

[Christian Dywan]

Created attachment 18194 [details]
Updated

I would like to move on with this, so I'm updating the patch. Opinions welcome.

[Maciej Stachowiak]

The code itself looks good to me (though I'm not fresh on Gtk+ programming) but I think there's a needed API design decision here, whether to copy the Mac port's separate settings object, or expose GObject properties right on the view, or both.

[Alp Toker]

Comment on attachment 18194 [details]
Updated

Thanks Christian for getting this patch back in action.

There's been some good discussion over the merits of having settings on WebView vs. a separate settings object and I think we should go for the latter now.

So it's best to have these properties on WebKitWebSettings.

As you suggested, we can look into having them on both objects later on. There are a couple of techniques we can use to do this, but I don't think we should do it just yet.

[Adam Strzelecki]

Created attachment 19110 [details]
Site using CSS system colors in Safari 3.0.4 (OK)

[Adam Strzelecki]

Created attachment 19111 [details]
Site using CSS system colors in latest WebKit r30153 (BAD, most of elements are BLACK)

[Adam Strzelecki]

Shit... sorry stupid buzilla moved myself to next bug, I wanted create attachments.... sorry again.

[Adam Strzelecki]

Comment on attachment 19111 [details]
Site using CSS system colors in latest WebKit r30153 (BAD, most of elements are BLACK)

Sorry wrong bug, Bugzilla is moving me to next bug without asking so... wrrr

[Adam Strzelecki]

Comment on attachment 19110 [details]
Site using CSS system colors in Safari 3.0.4 (OK)

Sorry wrong bug, Bugzilla is moving me to next bug without asking so... wrrr

[Gustavo Noronha (kov)]

I believe this no longer applies, since we now have WebKitWebSettings.