Bug 237544 - Notifications appear empty in awesome window manager
Summary: Notifications appear empty in awesome window manager
Status: RESOLVED FIXED
Alias: None
Product: konversation
Classification: Applications
Component: notifications (show other bugs)
Version: Git
Platform: Debian testing Linux
: NOR normal
Target Milestone: ---
Assignee: Konversation Developers
URL:
Keywords:
: 265176 (view as bug list)
Depends on:
Blocks:
 
Reported: 2010-05-13 21:27 UTC by Thomas Koch
Modified: 2011-02-02 21:53 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Thomas Koch 2010-05-13 21:27:39 UTC
Version:           1.2.3 (using KDE 4.4.3)
OS:                Linux
Installed from:    Debian testing/unstable Packages

After upgrading to KDE 4.4.3 I happily noticed that KMail now notifies me about new emails over the freedesktop notification system. However there were already some empty notifications without text.
I tracked these empty notifications down to kopete and konversation. The only difference to the KMail notifications was, that those from kopete and konversation were encapsulated in <qt> tags.
Trying with notify-send from the commandline, I noticed, that a message encapsulated with <qt> is indeed not shown but only an empty message.

I'll now also fill a bug report on awesome that it should tolerate unknown tags. However AFAIK <qt> is not a valid notification tag and I wonder what it should mean?
Comment 1 Eike Hein 2010-05-13 21:32:00 UTC
Reassigned to KMail since this seems to have nothing to do with Konversation.
Comment 2 Thomas Koch 2010-05-16 18:21:26 UTC
Sorry, my first paragraph was a bit unclear: KMail is the one program that sends proper notifications.
Kopete and Konversation seem to make problems, because they encapsulate there notifications in <qt> tags. Please reassign.
Comment 3 Eike Hein 2010-05-17 04:25:52 UTC
Sorry, I read your report a bit too hurriedly and parsed it as "KMail notifications for new mails don't show". Reassigning.

The <qt> tag wraps a Qt rich-text document, as such it is basically a synonym for <html>. This makes the HTML entities for the points brackets work, because otherwise "<nick>" might be parsed as a tag and omitted:

KNotification::event(QString::fromLatin1("message"), QString("<qt>&lt;%1&gt; %2</qt>").arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);

At least it was necessary in Qt 3. The Qt 4 documentaton on the matter[1] has a "Provided for compatibility with earlier versions of Qt." note on the <qt> tag that suggests it might be preferred to use <html> now, and perhaps Awesome would handle that better.

We'll look into what KDE's notification experts recommend on the matter.

1 = http://doc.qt.nokia.com/4.6/richtext-html-subset.html
Comment 4 Eike Hein 2010-05-17 04:48:42 UTC
commit fa49cee5e4804387ca212b55375accb62e5e7332
Author: Eike Hein <hein@kde.org>
Date:   Mon May 17 04:46:48 2010 +0200

    Replace <qt> tags in our KNotification event paylods with <html> tags.
    
    The <qt> was introduced to denote a Qt rich-text document, and used to
    be necessary to make the HTML entities we substitute for the pointy
    brackets around nicknames work. Using HTML entities was necessary to
    avoid the nicknames getting parsed as tags and dropped as unknown. How-
    ever, KNotification now forwards popup requests to freedesktop.org-
    compliant frontends via D-Bus, and those frontends may not be implemen-
    ted using Qt and thus can't handle the <qt> tag (an example of this is
    apparently the Awesome window manager). KNotify also makes no attempt
    to filter them from the payload. According to the Qt 4 documentation,
    <qt> is now a synonym for <html> that is provided for backward compati-
    bility. Thus, we now use <html>. This was also adviced by KNotify main-
    tainer Olivier Goffart.
    
    BUG:237544
    CCMAIL:ogoffart@kde.org

diff --git a/ChangeLog b/ChangeLog
index 90b2660..1556aec 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -113,6 +113,10 @@ Changes since 1.2.3:
 * The frame that used to be around the main window's tab widget when
   the tab bar was located either at the bottom or top position has
   been removed.
+* Improved compatibility with freedesktop.org-compliant notification
+  frontends other than KDE's. Other frontends could previously show
+  empty notification message contents due to non-standard content in
+  the messages.
 
 
 Changes from 1.2.2 to 1.2.3:
diff --git a/src/notificationhandler.cpp b/src/notificationhandler.cpp
index a86c709..dfef708 100644
--- a/src/notificationhandler.cpp
+++ b/src/notificationhandler.cpp
@@ -48,7 +48,7 @@ namespace Konversation
         QString cleanedMessage = removeIrcMarkup(message);
         QString forKNotify = Qt::escape(cleanedMessage);
 
-        KNotification::event(QString::fromLatin1("message"), QString("<qt>&lt;%1&gt; %2</qt>").arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
+        KNotification::event(QString::fromLatin1("message"), QString("<html>&lt;%1&gt; %2</html>").arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
 
         if(!Preferences::self()->trayNotifyOnlyOwnNick())
         {
@@ -76,7 +76,7 @@ namespace Konversation
         QString cleanedMessage = removeIrcMarkup(message);
         QString forKNotify = Qt::escape(cleanedMessage);
 
-        KNotification::event(QString::fromLatin1("nick"), QString("<qt>&lt;%1&gt; %2</qt>").arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
+        KNotification::event(QString::fromLatin1("nick"), QString("<html>&lt;%1&gt; %2</html>").arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
 
         startTrayNotification(chatWin);
 
@@ -102,7 +102,7 @@ namespace Konversation
         QString cleanedMessage = removeIrcMarkup(message);
         QString forKNotify = Qt::escape(cleanedMessage);
 
-        KNotification::event(QString::fromLatin1("queryMessage"), QString("<qt>&lt;%1&gt; %2</qt>").arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
+        KNotification::event(QString::fromLatin1("queryMessage"), QString("<html>&lt;%1&gt; %2</html>").arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
 
         startTrayNotification(chatWin);
 
@@ -318,9 +318,9 @@ namespace Konversation
         QString forKNotify = Qt::escape(cleanedMessage);
 
         if(fromNick.isEmpty())
-            KNotification::event(QString::fromLatin1("highlight"), QString("<qt>(%1) *** %2</qt>").arg(chatWin->getName()).arg(forKNotify), QPixmap(), m_mainWindow);
+            KNotification::event(QString::fromLatin1("highlight"), QString("<html>(%1) *** %2</html>").arg(chatWin->getName()).arg(forKNotify), QPixmap(), m_mainWindow);
         else
-            KNotification::event(QString::fromLatin1("highlight"), QString("<qt>(%1) &lt;%2&gt; %3</qt>").arg(chatWin->getName()).arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
+            KNotification::event(QString::fromLatin1("highlight"), QString("<html>(%1) &lt;%2&gt; %3</html>").arg(chatWin->getName()).arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
 
         if(Preferences::self()->oSDShowOwnNick() &&
             (!m_mainWindow->isActiveWindow() || (chatWin != m_mainWindow->getViewContainer()->getFrontView())))
Comment 5 Thomas Koch 2010-05-17 08:38:47 UTC
Will KNotify filter out the html tag before forwarding the notification text to notification-daemon (which is used by awesome and gnome AFAIK)?
If not, awesome will still not show the text because it simply forwards it to pango without any modification.

This spec does not mention any encapsulating tag for the notification:
http://www.galago-project.org/specs/notification/0.9/index.html

So I believe, it'd still be a bug to surround the text with <html>. If KNotify however requires this, then KNotify should strip the surrounding <html> tag before forwarding.
Comment 6 Eike Hein 2010-05-17 08:43:45 UTC
Perhaps this should be reassigned to KNotify, then. Oliver, could you chime in?

I'm not sure if the Galago spec is authoritative, though, or if there's a newer one around somewhere - afaik KDE used Galago as a starting point but fixed it up / extended it, and that went back to a fd.o spec in a git repo somewhere. I kind of lost track of all that though.
Comment 7 Eike Hein 2010-05-17 08:50:26 UTC
Just as some additional info: KNotify also works when there is no freedesktop.org notification frontend to forward to; the popup notification type is then displayed in a standard Qt dialog box, which I think definitely needs the <html>/<qt> thing. Of course by virtue of KDE 4's own desktop shell implementing the D-Bus notification interface this is a corner case, but it exists. And the application calling KNotification shouldn't have to care, of course.

BTW, if Awesome just forwards the text to Pango, how are the HTML tags that the spec does specify need to work, like <img>, handled? If Pango handles basic HTML, why does it choke on <html>?
Comment 8 Eike Hein 2010-07-01 16:19:37 UTC
commit fa49cee5e4804387ca212b55375accb62e5e7332
Author: Eike Hein <hein@kde.org>
Date:   Mon May 17 04:46:48 2010 +0200

    Replace <qt> tags in our KNotification event paylods with <html> tags.
    
    The <qt> was introduced to denote a Qt rich-text document, and used to
    be necessary to make the HTML entities we substitute for the pointy
    brackets around nicknames work. Using HTML entities was necessary to
    avoid the nicknames getting parsed as tags and dropped as unknown. How-
    ever, KNotification now forwards popup requests to freedesktop.org-
    compliant frontends via D-Bus, and those frontends may not be implemen-
    ted using Qt and thus can't handle the <qt> tag (an example of this is
    apparently the Awesome window manager). KNotify also makes no attempt
    to filter them from the payload. According to the Qt 4 documentation,
    <qt> is now a synonym for <html> that is provided for backward compati-
    bility. Thus, we now use <html>. This was also adviced by KNotify main-
    tainer Olivier Goffart.
    
    BUG:237544
    CCMAIL:ogoffart@kde.org

diff --git a/ChangeLog b/ChangeLog
index 90b2660..1556aec 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -113,6 +113,10 @@ Changes since 1.2.3:
 * The frame that used to be around the main window's tab widget when
   the tab bar was located either at the bottom or top position has
   been removed.
+* Improved compatibility with freedesktop.org-compliant notification
+  frontends other than KDE's. Other frontends could previously show
+  empty notification message contents due to non-standard content in
+  the messages.
 
 
 Changes from 1.2.2 to 1.2.3:
diff --git a/src/notificationhandler.cpp b/src/notificationhandler.cpp
index a86c709..dfef708 100644
--- a/src/notificationhandler.cpp
+++ b/src/notificationhandler.cpp
@@ -48,7 +48,7 @@ namespace Konversation
         QString cleanedMessage = removeIrcMarkup(message);
         QString forKNotify = Qt::escape(cleanedMessage);
 
-        KNotification::event(QString::fromLatin1("message"), QString("<qt>&lt;%1&gt; %2</qt>").arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
+        KNotification::event(QString::fromLatin1("message"), QString("<html>&lt;%1&gt; %2</html>").arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
 
         if(!Preferences::self()->trayNotifyOnlyOwnNick())
         {
@@ -76,7 +76,7 @@ namespace Konversation
         QString cleanedMessage = removeIrcMarkup(message);
         QString forKNotify = Qt::escape(cleanedMessage);
 
-        KNotification::event(QString::fromLatin1("nick"), QString("<qt>&lt;%1&gt; %2</qt>").arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
+        KNotification::event(QString::fromLatin1("nick"), QString("<html>&lt;%1&gt; %2</html>").arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
 
         startTrayNotification(chatWin);
 
@@ -102,7 +102,7 @@ namespace Konversation
         QString cleanedMessage = removeIrcMarkup(message);
         QString forKNotify = Qt::escape(cleanedMessage);
 
-        KNotification::event(QString::fromLatin1("queryMessage"), QString("<qt>&lt;%1&gt; %2</qt>").arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
+        KNotification::event(QString::fromLatin1("queryMessage"), QString("<html>&lt;%1&gt; %2</html>").arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
 
         startTrayNotification(chatWin);
 
@@ -318,9 +318,9 @@ namespace Konversation
         QString forKNotify = Qt::escape(cleanedMessage);
 
         if(fromNick.isEmpty())
-            KNotification::event(QString::fromLatin1("highlight"), QString("<qt>(%1) *** %2</qt>").arg(chatWin->getName()).arg(forKNotify), QPixmap(), m_mainWindow);
+            KNotification::event(QString::fromLatin1("highlight"), QString("<html>(%1) *** %2</html>").arg(chatWin->getName()).arg(forKNotify), QPixmap(), m_mainWindow);
         else
-            KNotification::event(QString::fromLatin1("highlight"), QString("<qt>(%1) &lt;%2&gt; %3</qt>").arg(chatWin->getName()).arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
+            KNotification::event(QString::fromLatin1("highlight"), QString("<html>(%1) &lt;%2&gt; %3</html>").arg(chatWin->getName()).arg(fromNick).arg(forKNotify), QPixmap(), m_mainWindow);
 
         if(Preferences::self()->oSDShowOwnNick() &&
             (!m_mainWindow->isActiveWindow() || (chatWin != m_mainWindow->getViewContainer()->getFrontView())))
Comment 9 Sebastian Dörner 2010-08-26 20:33:18 UTC
Surrounding <qt> or <html> tags (not specified in the specs) don't lead to empty messages in awesome anymore, but prevent other markup (<i>, <b>) from being interpreted. This leads to ugly tags in the message.
The same happens when <img> or <a> are used, but these do not need to be supported according to the spec (although they should be filtered out in this case).
The corresponding bug in awesome window manager is
http://awesome.naquadah.org/bugs/index.php?do=details&task_id=765
Comment 10 Eike Hein 2011-02-02 20:08:57 UTC
*** Bug 265176 has been marked as a duplicate of this bug. ***
Comment 11 Peter Simonsson 2011-02-02 21:53:01 UTC
Git commit 2bfb6a10b2919ae8192e6d5b01ed733d7349b631 by Peter Simonsson.
Committed on 02/02/11 at 21:48.
Pushed by psn into branch 'master'.

Remove <html> tags from notification messages

KNotification does not seem to require them and they are not
according to galago spec.

BUG: 237544

M  +8    -8    src/notificationhandler.cpp     

http://commits.kde.org/konversation/2bfb6a10b2919ae8192e6d5b01ed733d7349b631