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 667628 - GtkWindow:application property should have the G_PARAM_CONSTRUCT flag
GtkWindow:application property should have the G_PARAM_CONSTRUCT flag
Status: RESOLVED NOTABUG
Product: gtk+
Classification: Platform
Component: Class: GtkApplication
unspecified
Other Linux
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks: 667619
 
 
Reported: 2012-01-10 15:02 UTC by Guillaume Desmottes
Modified: 2012-02-01 18:00 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
set the G_PARAM_CONSTRUCT flag on the GtkWindow:application property (1.20 KB, patch)
2012-01-10 15:03 UTC, Guillaume Desmottes
committed Details | Review

Description Guillaume Desmottes 2012-01-10 15:02:50 UTC
I have an object which is a subclass of GtkWindowApplication. I call gtk_application_set_menubar() in its constructed method. In order to do so, I need the window's application but gtk_window_get_application() returns NULL because, at this state, the property has not beet set yet. Setting the G_PARAM_CONSTRUCT flag solves this.
Comment 1 Guillaume Desmottes 2012-01-10 15:03:58 UTC
Created attachment 204944 [details] [review]
set the G_PARAM_CONSTRUCT flag on the GtkWindow:application property

This allows subclass to get the value of this property in their constructed
method.
Comment 2 Allison Karlitskaya (desrt) 2012-01-10 15:06:06 UTC
Review of attachment 204944 [details] [review]:

looks good, thanks
Comment 3 Guillaume Desmottes 2012-01-10 15:11:12 UTC
Attachment 204944 [details] pushed as d4fe912 - set the G_PARAM_CONSTRUCT flag on the GtkWindow:application property
Comment 4 Frederic Peters 2012-01-11 20:32:46 UTC
I don't get quite how it went, but this commit breaks epiphany/newmenus branch.

"""
GLib-GObject-CRITICAL **: g_object_ref: assertion `G_IS_OBJECT (object)' failed

GLib-GIO-CRITICAL **: g_action_group_list_actions: assertion `G_IS_ACTION_GROUP (action_group)' failed
"""

434	  for (i = 0; actions[i]; i++)
(gdb) bt full
  • #0 g_action_muxer_insert
    at gactionmuxer.c line 434
  • #1 gtk_application_window_real_realize
    at gtkapplicationwindow.c line 677
  • #2 g_cclosure_marshal_VOID__VOID
    at gmarshal.c line 85
  • #3 g_type_class_meta_marshal
    at gclosure.c line 885
  • #4 g_closure_invoke
    at gclosure.c line 774
  • #5 signal_emit_unlocked_R
    at gsignal.c line 3232
  • #6 g_signal_emit_valist
    at gsignal.c line 3033
  • #7 g_signal_emit
    at gsignal.c line 3090
  • #8 gtk_widget_realize
    at gtkwidget.c line 4405
  • #9 ephy_gui_window_update_user_time
    at ephy-gui.c line 397
  • #10 ephy_shell_new_tab_full
    at ephy-shell.c line 795
  • #11 session_command_dispatch
    at ephy-session.c line 728
  • #12 g_idle_dispatch
    at gmain.c line 4632
  • #13 g_main_dispatch
    at gmain.c line 2513
  • #14 g_main_context_dispatch
    at gmain.c line 3050
  • #15 g_main_context_iterate
    at gmain.c line 3121
  • #16 g_main_context_iteration
    at gmain.c line 3182
  • #17 g_application_run
    at gapplication.c line 1599
  • #18 main
    at ephy-main.c line 472

Comment 5 Allison Karlitskaya (desrt) 2012-01-11 22:45:55 UTC
probably because it is now being explicitly set to NULL on construct...
Comment 6 Xan Lopez 2012-01-12 11:46:29 UTC
(In reply to comment #5)
> probably because it is now being explicitly set to NULL on construct...

So is this a problem in GTK+ or should ephy be doing something?
Comment 7 Matthias Clasen 2012-01-12 13:18:18 UTC
How are you constructing your GtkApplicationWindow ?
Looks like you need to make sure that it has a nonzero application set before you realize it.
Comment 8 Matthias Clasen 2012-01-12 13:20:28 UTC
if we wanted to make late setting of the application on applicationwindows possible, we would have to add a bunch of NULL checks, and listen to notify::application to update the muxer when the application appears.
Comment 9 Xan Lopez 2012-01-12 18:11:28 UTC
Seems the issue is that I was doing gtk_application_add_window but not the opposite (like, passing the application property on construction to the window). That seems to fix things. Are both needed or can I get rid of gtk_application_add_window?
Comment 10 Carlos Garnacho 2012-01-12 21:40:34 UTC
This commit also seems to break some 3.2 apps, I've spotted eog and ephy so far
Comment 11 Carlos Garnacho 2012-01-12 21:47:02 UTC
For those, the main window seems to set the application on init(), but it's then unset on construct(). This holds, then releases the application, so it just goes through g_application_run()
Comment 12 Matthias Clasen 2012-01-12 23:02:55 UTC
passing the app to the applicationwindow constructor will end up calling
gtk_window_set_application, which will in turn call gtk_application_add_window.
So you should be good without a manual add_window
Comment 13 Allison Karlitskaya (desrt) 2012-01-13 08:04:07 UTC
I've been thinking about this a bit lately...

We originally needed "application" to be writable-after-init so that gtk_window_set_application() and gtk_application_add_window() could work.

It's clear that we never want a GtkApplicationWindow to change applications, however -- and it takes the application as a construct-time parameter.

I think a decent compromise here (and one that I've been assuming internally without documenting) is that the application will never be changed while the window is realized.

Of course, that doesn't help with solving this bug...
Comment 14 Allison Karlitskaya (desrt) 2012-01-13 08:15:03 UTC
(In reply to comment #9)
> Seems the issue is that I was doing gtk_application_add_window but not the
> opposite (like, passing the application property on construction to the
> window). That seems to fix things. Are both needed or can I get rid of
> gtk_application_add_window?

gtk_application_add_window() and gtk_window_set_application() will call each other.  They are more or less completely equivalent.

Similarly, set_application(NULL) or to a different application will call gtk_application_remove_window() internally.
Comment 15 Allison Karlitskaya (desrt) 2012-01-17 15:49:36 UTC
Bug 425324 for a bit of a history lesson.  In the original patch there, the 'constructed' vfunc wasn't called until after all the properties were set.  Tim reworked it a bit to call 'constructed' before the non-construct properties and I failed to notice at the time...

So as it stands today, this is roughly the order of places visited during the construction of an object:

  - init
  - construct properties set (including to default values)
  - constructed vfunc
  - non-construct properties set

cassidy's problem was caused by the reworking that happened in bug 425324 -- non-construct properties not being accessible to the 'constructed' function.  We fix that by making it a construct property, but that has a side effect:

Construct properties *always* have their setters called, after init, to their default values.  In this case, that's NULL.  So if you set the property to something in _init, you can expect it to be blown away.

This is what eog does.



So what can we do?

  - revisit bug 425324 (which would probably cause a lot more similar
    breakages in lots of existing code)

  - revert this patch and say that the application is not available in
    'constructed' despite the fact that gtk_application_window_new takes
    it as an argument

  - fix eog (i'm starting not to like this option...)

  - make 'application' a construct property only for GtkApplicationWindow
    and not normal GtkWindow

  - have our property-based object construction taken out back and shot
Comment 16 Allison Karlitskaya (desrt) 2012-01-17 15:57:35 UTC
I've reverted the original patch to unbreak eog.  I think we need to figure out a way to proceed that is back-compat.
Comment 17 Allison Karlitskaya (desrt) 2012-02-01 18:00:18 UTC
After understanding the original nature of the request that caused this bug to be filed, I think it's actually not an issue at all.  Nothing broken, nothing to fix.