After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 608802 - Setting "type" after construct fails with new glib
Setting "type" after construct fails with new glib
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
2.28.x
Other Linux
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2010-02-02 15:27 UTC by Pascal Terjan
Modified: 2010-02-08 17:44 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Set the window type in g_object_new (620 bytes, patch)
2010-02-02 15:27 UTC, Pascal Terjan
none Details | Review
[ShellEmbeddedWindow] Set window type in g_object_new (1.17 KB, patch)
2010-02-02 16:32 UTC, Florian Müllner
none Details | Review
[ShellEmbeddedWindow] Set window type in g_object_new (3.18 KB, patch)
2010-02-07 15:27 UTC, Florian Müllner
committed Details | Review

Description Pascal Terjan 2010-02-02 15:27:44 UTC
Created attachment 152840 [details] [review]
Set the window type in g_object_new

Log level 16: IA__g_object_set_valist: construct property "type" for object `ShellEmbeddedWindow' can't be set after construction
Comment 1 Colin Walters 2010-02-02 16:31:51 UTC
Review of attachment 152840 [details] [review]:

This isn't strictly speaking correct from a binding perspective; I'm not sure if we make any Shell.EmbeddedWindow from script at the moment (probably not), but bindings don't use the C convenience constructors.

I believe the correct fix is to define a constructor for ShellEmbeddedWindow, and call g_object_set in there.  But I can't find a good reference on the change right now.
Comment 2 Colin Walters 2010-02-02 16:31:51 UTC
Review of attachment 152840 [details] [review]:

This isn't strictly speaking correct from a binding perspective; I'm not sure if we make any Shell.EmbeddedWindow from script at the moment (probably not), but bindings don't use the C convenience constructors.

I believe the correct fix is to define a constructor for ShellEmbeddedWindow, and call g_object_set in there.  But I can't find a good reference on the change right now.
Comment 3 Florian Müllner 2010-02-02 16:32:06 UTC
Created attachment 152847 [details] [review]
[ShellEmbeddedWindow] Set window type in g_object_new

Merci beaucoup - patch looks good.

I replaced the original attachment with a git-formatted patch - I set author name/email according to your bugzilla account, which I hope is OK with you ...
Comment 4 Pascal Terjan 2010-02-02 16:34:01 UTC
OK sorry for sending git-diff and not committing first
Comment 5 Owen Taylor 2010-02-02 16:50:02 UTC
For reference IRC Conversation from Dec 6 on this topic with Alex Larsson; not sure there's anything that useful in it. My expressed opinion at that point was doing it in the _new() function is conceptually wrong, but probably way easier than the alternatives, since we are only creating ShellEmbeddedWindow from C.

If we do it in the new() function, we absolutely do need a comment saying what's going on and why we are doing it there.

<alex>  owen: so, i just got gnome-shell running on my desktop
<alex>  owen: And i ran into a snag with one of the patches from the gobject-performance branch
<alex>  I get this warning:
<alex>  I get this warning:
<alex>  Window manager warning: Log level 16: IA__g_object_set_valist: construct property "type" for object `ShellEmbeddedWindow' can't be set after construction
<alex>  The cause is shell_embedded_window_init () calling g_object_set on "type" in the init
 function
<alex>  versus https://bugzilla.gnome.org/show_bug.cgi?id=557151
 *       alex reopened it and added a comment
<alex>  owen: does it actually manage to set type to popup, or is that overwritten later?
<owen>  alex: It might not - don't know if there woudl be any observable differences between 
a popup and not
<alex>  could you not see it with xprop?
<owen>  I think the main reason I set it to be a popup was to avoid the round-trips on allocation
<owen>  alex: yes, you could
<owen>  (by observable I don't mean that it would be identical, but rather that I wouldn't have noticed if it wasn't working)
<alex>  ah, yeah
<alex>  owen: but the real place to set it would be in the new call, wouldn't it?
<owen>  well, it could be done there if it doesn't work
<alex>  It might be that we're mistaken and the performance patch is wrong.
<owen>  it would be nice if it *was* possible to chnage defaults in a subclass in an encapsulated way without some horrible workaround with a constructor(), but in this particular case, we're only constructing the class through the new() function
<owen>  alex: my gir-repository build is broken at the moment and I want to go out and enjoy a nice synny and snowy morning, but seems like checking if the init() call actually does work without those patches woudl be the first step
<alex>  owen: maybe you can re-register the property?
<alex>  although that is pretty lame...
<alex>  owen: lemme test it
<alex>  which windows are shell-embeded-window?
<owen>  alex: the windows the tray icons are in - so if you xwininfo -tree -root and look up hmm, maybe two windows from the toplevels of the tray icons there should be another set of toplevels, which are the ShellEmbeddedWindows, and those are reparented into the overview window
Comment 6 Florian Müllner 2010-02-07 15:27:19 UTC
Created attachment 153198 [details] [review]
[ShellEmbeddedWindow] Set window type in g_object_new

Use a custom constructor, so that the window type property is set correctly
for both the C convenience constructor and bindings.
Comment 7 Florian Müllner 2010-02-08 17:44:09 UTC
Attachment 153198 [details] pushed as fb7ed1e - [ShellEmbeddedWindow] Set window type in g_object_new