<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="8" style="border: 1px #c9c399 solid;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="http://reviewboard.kde.org/r/4555/">http://reviewboard.kde.org/r/4555/</a>
</td>
</tr>
</table>
<br />
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="8" style="background-image: url('http://reviewboard.kde.orgrb/images/review_request_box_top_bg.png'); background-position: left top; background-repeat: repeat-x; border: 1px black solid;">
<tr>
<td>
<div>Review request for kdelibs, Olivier Goffart and Aurélien Gâteau.</div>
<div>By Sjors Gielen.</div>
<p style="color: grey;"><i>Updated 2010-07-09 11:55:56.993625</i></p>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Changes</h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0;">I added a bug that seemed related, but it in fact doesn't seem to be. This might confuse people, so I'm removing it from this review request.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Testing </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0;">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.</pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>/trunk/KDE/kdebase/runtime/knotify/notifybypopup.h <span style="color: grey">(1147918)</span></li>
<li>/trunk/KDE/kdebase/runtime/knotify/notifybypopup.cpp <span style="color: grey">(1147918)</span></li>
</ul>
<p><a href="http://reviewboard.kde.org/r/4555/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>