Yes, of course.

This is already the plan, but we are waiting for desktop usage of GTK+ 2.10. This should be in Ubuntu Edgy or FC6 I think.

It would be nice to finish breaking out the tray stuff into a separate file from gossip-app.c at the same time.

*** Bug 349296 has been marked as a duplicate of this bug. ***

Created attachment 70951 [details] [review]
first patch

Here is a first patch. I didn't split the code to another file yet.

Any comment ?

#1:  Please try not to include other patches in different bug patches:

    @@ -932,7 +916,8 @@ app_main_window_delete_event_cb (GtkWidg
 	    			 GdkEvent  *event,
     				 GossipApp *app)
     {
    -	return app_main_window_quit_confirm (app, window);
    +	gossip_app_set_visibility (FALSE);
    +	return TRUE;


#2: I would rather NOT change this API: gossip_notify_set_widget() ---> gossip_notify_set_status_icon(), because you can attach notifications to any widget.

Other than that, I got most of the way through it, but it overlaps in places and I would have to see the source after applying the patch. It looks good so far.

Richard: it looks like we are removing a lot of crap with this patch (which I suspected) is there anything in particular you wanted in the new gossip-tray.c?

Created attachment 70962 [details] [review]
updated patch

Thanks for reviewing.

#1 Oops, sorry ! Reversed this patch.

#2 GtkStatusIcon isn't a GtkWidget but a GObject. So I added a new gossip_notify_set_attach_status_icon() and we can now choose to attach to the StatusIcon or a Widget. For now the attach_widget isn't used but it can be used in the future.

In previous patch I forgot to replace gossip_have_tray() by gtk_status_icon_is_embedded(), it's now done. I also forgot to remove some eggs from the Makefile.am.

We might not need a gossip-tray I guess. It's just that there were so much logic in -app that didn't really belong there. We can look at that later if needed, and just get rid of the bad eggs now.

Created attachment 71029 [details] [review]
Use gtk_status_icon_position_menu

I didn't see this useful function. The popup menu is now better positioned, it is always on the bottom of the icon and never overlaps it.

Note that in the latest libnotify version there is support for positioning the bubbles close to the status icon. We need to depend on that as well and use it.

I already use notify_notification_attach_to_status_icon() if that's what you're talking about.

Yes, great!

*** Bug 355016 has been marked as a duplicate of this bug. ***

Any chance you could redo this patch? It all applies except for gossip-app.c and some simple header file issue in gossip-notify.h.

HEAD now has GTK+2.10 deps so we can commit this as soon as it is ready.

Sorry about this, but this is why I tend to wait before writing patches ahead of time.

Created attachment 73098 [details] [review]
updated patch

updated to last HEAD. I also bumped the dependency for libnotify to 0.4.1 because notify_notification_attach_to_status_icon() is a new symbol is this release.

I get warnings on ubuntu edgy that I reported at bug #356941, I don't see how it can be a gossip bug...

Committed, thanks Xavier!