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 571177 - NotificationArea should check for 'actions' capability before setting actions [PATCH]
NotificationArea should check for 'actions' capability before setting actions...
Status: RESOLVED FIXED
Product: banshee
Classification: Other
Component: User Interface
git master
Other Linux
: Normal minor
: 1.x
Assigned To: Banshee Maintainers
Banshee Maintainers
Depends on:
Blocks:
 
 
Reported: 2009-02-10 14:01 UTC by Cody Russell
Modified: 2009-05-15 11:02 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Patch (1.28 KB, patch)
2009-02-10 14:02 UTC, Cody Russell
none Details | Review
Cleaner patch (801 bytes, patch)
2009-02-10 14:05 UTC, Cody Russell
committed Details | Review
Revised patch (1.68 KB, patch)
2009-02-25 15:20 UTC, Cody Russell
committed Details | Review

Description Cody Russell 2009-02-10 14:01:56 UTC
Banshee's notification area should check that the notification daemon supports actions before it sets actions.
Comment 1 Cody Russell 2009-02-10 14:02:59 UTC
Created attachment 128375 [details] [review]
Patch
Comment 2 Cody Russell 2009-02-10 14:05:58 UTC
Created attachment 128376 [details] [review]
Cleaner patch
Comment 3 Andrew Conkling 2009-02-10 15:27:25 UTC
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?
Comment 4 Ted Gould 2009-02-10 15:53:55 UTC
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.
Comment 5 Gabriel Burt 2009-02-10 20:00:31 UTC
Looks good, please commit.
Comment 6 Cody Russell 2009-02-10 20:06:18 UTC
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)
Comment 7 Gabriel Burt 2009-02-10 20:07:55 UTC
Thanks Cody
Comment 8 Cody Russell 2009-02-25 15:20:33 UTC
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.
Comment 9 Bertrand Lorentz 2009-02-25 18:36:56 UTC
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.
Comment 10 Chow Loong Jin 2009-02-26 10:41:08 UTC
Then perhaps it's time to sync the in-tree notify-sharp with the official one?
Comment 11 Cody Russell 2009-03-02 03:09:47 UTC
Why is there an in-tree one anyway?
Comment 12 Chow Loong Jin 2009-03-02 06:37:51 UTC
Probably for distros which do not package it. It was relevant back then, but probably not any more.
Comment 13 Bertrand Lorentz 2009-05-14 18:49:04 UTC
Gabriel committed the patch and I removed the in-tree notify-sharp, all in git master. So I think we're OK here.

Thanks !
Comment 14 Bertrand Lorentz 2009-05-15 11:02:58 UTC
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).