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 621584 - notification area crashes with multiple screen setup
notification area crashes with multiple screen setup
Status: RESOLVED FIXED
Product: gnome-panel
Classification: Other
Component: notification area
2.30.x
Other Linux
: Normal normal
: ---
Assigned To: Panel Maintainers
Panel Maintainers
: 346162 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2010-06-14 20:18 UTC by Ray Strode [halfline]
Modified: 2010-06-22 17:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Defer tray setup until after it's added to panel (5.25 KB, patch)
2010-06-14 20:18 UTC, Ray Strode [halfline]
none Details | Review
Same as attachment 163626 but for master (5.25 KB, patch)
2010-06-14 20:20 UTC, Ray Strode [halfline]
committed Details | Review
patch against gnome-2-30 (4.92 KB, patch)
2010-06-22 15:22 UTC, Ray Strode [halfline]
committed Details | Review

Description Ray Strode [halfline] 2010-06-14 20:18:53 UTC
Created attachment 163626 [details] [review]
Defer tray setup until after it's added to panel

Right now, the notification area doesn't handle multi-screen (old style "zaphod mode" setups i mean) configurations very well.  It just nose dives when trying to add a notification area to a panel on the second screen.

This is because it calls gtk_widget_get_screen on the applet before the applet is added to the panel, which means gtk_widget_get_screen is returning invalid information.

The above patch just deferrs most of the guts until after the applet is realized.
Comment 1 Ray Strode [halfline] 2010-06-14 20:20:05 UTC
Created attachment 163627 [details] [review]
Same as attachment 163626 [details] [review] but for master

This is an untested forward port of the patch to master
Comment 2 Vincent Untz 2010-06-22 02:46:50 UTC
Hrm, both patches are for master, it seems. Do you have the 2.30 one available somewhere? :-)
Comment 3 Vincent Untz 2010-06-22 02:50:02 UTC
Review of attachment 163627 [details] [review]:

Thanks for finding this! It's been a long-standing bug, and I see now that I was just not looking at the right place!

::: applets/notification_area/main.c
@@ -273,3 @@
-  g_signal_connect (applet, "destroy",
-		    G_CALLBACK (applet_destroy), data);
-

Why are we moving this? Is it to avoid checks that applet is realized in the callbacks?

@@ -295,3 @@
-  g_free (ui_path);
-  g_object_unref (action_group);
-

I don't think this needs to be moved.
Comment 4 Vincent Untz 2010-06-22 03:19:17 UTC
*** Bug 346162 has been marked as a duplicate of this bug. ***
Comment 5 Vincent Untz 2010-06-22 11:44:30 UTC
(In reply to comment #3)
> @@ -295,3 @@
> -  g_free (ui_path);
> -  g_object_unref (action_group);
> -
> 
> I don't think this needs to be moved.

(I was commenting on the whole block, not just the last two lines, obviously)
Comment 6 Ray Strode [halfline] 2010-06-22 15:20:51 UTC
Review of attachment 163627 [details] [review]:

::: applets/notification_area/main.c
@@ -273,3 @@
-  g_signal_connect (applet, "destroy",
-		    G_CALLBACK (applet_destroy), data);
-

Those callbacks take the AppletData struct as user data.  It isn't allocated until realization now.

@@ -295,3 @@
-  g_free (ui_path);
-  g_object_unref (action_group);
-

At least the add_actions call does, since it takes the data argument that's now allocated in realize.

We could move the g_slice_new call back and just fully initialize it later, but there's also no advantage to creating the action group early afaict.
Comment 7 Ray Strode [halfline] 2010-06-22 15:22:11 UTC
Created attachment 164313 [details] [review]
patch against gnome-2-30
Comment 8 Vincent Untz 2010-06-22 17:26:50 UTC
Thanks Ray!