Created attachment 128375 [details] [review]
Patch

Created attachment 128376 [details] [review]
Cleaner patch

I'm sure you know something that I don't, but what's the problem if the daemon doesn't support this? Will the buttons do nothing?

It of course depends on the implementation of the daemon.  In some cases it may return error instead of showing the notification.  In the case that we're specifically interested in the notification will be shown in a degrated "compatibility mode" to allow for applications that aren't checking.  But, since it will be a degrated experience we're trying to patch as many apps as possible to have the best experience.

Thank you.

Looks good, please commit.

2009-02-10  Cody Russell  <bratsche@gnome.org>

        * src/Extensions/Banshee.NotificationArea/Notifications/Notification.cs:
        When adding a new action, first check that the notification daemon 
        supports actions. (BGO #571177)

Thanks Cody

Created attachment 129490 [details] [review]
Revised patch

So, the previous patch modified Notification.AddAction() directly.  Now in retrospect this was not correct.. that seems to be an in-tree notify-sharp, and when we applied this to Jaunty it didn't seem to work because there is a system installed notify-sharp that was being used instead.

Applying this patch to the system's notify-sharp is not a good solution.  Right now if your application uses actions in the OSDNotify server, it very deliberately pops up an annoying dialog (with the actions, so you don't lose functionality).  The main purpose of this is to make it easier to find applications that may need to revise the way they're doing notifications. 

Also, there may be notifications that ONLY make sense if they have actions in them and it may be preferred that they display nothing at all than display a notification without an action.  So patch notify-sharp is a problem in this case.

I'm submitting this revised patch that has Banshee check the notification server for actions.  If it's accepted I'll revert the previous patch and then apply this one.

With the patch, compilation fails when using the in-tree copy of notify-sharp :

Compiling Banshee.NotificationArea.dll...
./Banshee.NotificationArea/NotificationAreaService.cs(208,67): error CS0234: The type or namespace name `Global' does not exist in the namespace `Notifications'. Are you missing an assembly reference?
Compilation failed: 1 error(s), 0 warnings

To reproduce this easily, just move away your notify-sharp.pc file, re-run ./configure and make.

Also, please use spaces for indentation.

Then perhaps it's time to sync the in-tree notify-sharp with the official one?

Why is there an in-tree one anyway?

Probably for distros which do not package it. It was relevant back then, but probably not any more.

Gabriel committed the patch and I removed the in-tree notify-sharp, all in git master. So I think we're OK here.

Thanks !

For the record :
I re-added the in-tree notify-sharp and updated it to the latest svn.

The bug is fixed in both situations (external or in-tree).