Review Request: Ensure notifications sent to DBus notification daemons are compatible with that daemon's capabilities

[Sjors Gielen]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviewboard.kde.org/r/4555/
-----------------------------------------------------------

(Updated 2010-07-09 20:11:51.096617)


Review request for kdelibs, Olivier Goffart and Aurélien Gâteau.


Changes
-------

ensurePopupCompatibility now returns a new KNotifyConfig* object, it does not modify the old one anymore. Changed the code in NotifyByPopup to reflect that. (Fixed FTBFS too.)


Summary
-------

KNotify supports sending notifications to a DBus notification daemon, but it currently does not make sure the notifications it sends are compatible with that specific daemon. More concretely,  notify-osd by design does not support HTML bodies or 'actions' (clickable buttons). It reports it does not support them, when you ask for its capabilities; however, knotify does not currently do that. KNotify then sends actions and HTML markup anyway, resulting in notify-osd displaying an ugly focus-stealing dialog, sometimes even with plaintext HTML entities in them.

This patch implements asking the notification daemon for capabilities. If a capability is not supported, its use is removed before sending the notification. There are no public API changes.

I would like to thank Aurélien Gateau for the capability retrieving parts of this patch (they were in another one of his patches, but those got rejected earlier because of the general goal of that patchset). If this patch is accepted, I would like it to be in the KDE 4.5 release, primarily because it makes KDE applications that use KNotify usable on Ubuntu again.


Diffs (updated)
-----

  /trunk/KDE/kdebase/runtime/knotify/knotifyconfig.h 1147918 
  /trunk/KDE/kdebase/runtime/knotify/knotifyconfig.cpp 1147918 
  /trunk/KDE/kdebase/runtime/knotify/notifybypopup.h 1147918 
  /trunk/KDE/kdebase/runtime/knotify/notifybypopup.cpp 1147918 

Diff: http://reviewboard.kde.org/r/4555/diff


Testing
-------

The patch compiles and works on my Mac. I have quickly written a notification daemon of my own, and changing the capabilities directly changes the way the notifications are sent. I haven't tested what happens when a notification server does not implement the GetCapabilities call, but if that happens the code should assume that the daemon does not support anything and it will strip everything off.

The code itself is only tested on Mac OS X, running my own notification daemon. I expect it to work on Gnome with notify-osd, and if Plasma implements GetCapabilities correctly, behavior should not change there. Testing on those platforms is still required.


Thanks,

Sjors

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20100709/1b52d488/attachment.htm>