[Webkit-unassigned] [Bug 19130] ChromeClient::createWindow and friends need to be implemented

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon May 26 09:06:14 PDT 2008


http://bugs.webkit.org/show_bug.cgi?id=19130





------- Comment #6 from marco.barisione at collabora.co.uk  2008-05-26 09:06 PDT -------
(In reply to comment #5)
> Created an attachment (id=21341)
 --> (http://bugs.webkit.org/attachment.cgi?id=21341&action=view) [edit]
> patch merged with work by me and barisione

Some comments on the patch:

- In webkit_web_view_real_create_web_view() and
webkit_web_view_real_show_web_view() you can omit param names as they are not
used.

- It's not clear from the documentation who is the owner of the object returned
by "create-web-view". IMHO we are doing the right thing without calling
g_object_ref in the callback as the reference will be owned by the window, but
then we have to document this and to get the reference count right (i.e. use
g_value_take object in the accumulator/unref the value returned by the emission
of the signal).

- Use the exact type when registering new signals, not just G_TYPE_OBJECT.

- How about replacing locationbar with uribar? We use uri in every other place,
but I don't know what I prefer. In both cases I would prefer to separate bar
from the previous word but then it wouldn't be consistent with the other names.

- The default value for resizable should be TRUE.

- The default value for fullscreen should be FALSE.

- The default value for dialog should be FALSE.

- WebKitWebFeatures doesn't have functions to access directly properties but
probably it's better this way than adding a lot of getters/setters.

- g_object_notify() is needed only in functions setting values not in the
implementation of set_property as g_object_set_property already emits it.

- Change the _set_window_features() function to set it only if it's different
from the previous one and to use g_object_notify() after setting it. I know
that other properties are not doing it but there is a bug open about this (see
bug #18405).


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



More information about the webkit-unassigned mailing list