[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    [knotifications] /: Add unit tests for KNotification and fix issues uncovered by them
From:       Martin Klapetek <mklapetek () kde ! org>
Date:       2016-03-31 2:50:28
Message-ID: E1alSgm-00031v-Kq () scm ! kde ! org
[Download RAW message or body]

Git commit d4dd58759284ffe09420664e04a7a31e338e9ff1 by Martin Klapetek.
Committed on 31/03/2016 at 02:45.
Pushed by mklapetek into branch 'master'.

Add unit tests for KNotification and fix issues uncovered by them

Adds basic set of unit tests including fake notifications server.
This helped uncover and fix these issues:

* Calling close() on KNotification that was not "sent" would not emit
closed() and would not delete it
* Closing a notification can delete the KNotification object prematurely
* Invoking an action leads to unnecessary dbus roundtrip
* Invoking an action would fail to properly close and delete the
KNotification object
* Start assigning the ids at 0 rather than 1 with -1 still being
invalid.

REVIEW: 127398

M  +1    -0    CMakeLists.txt
A  +11   -0    autotests/CMakeLists.txt
A  +79   -0    autotests/fake_notifications_server.cpp     [License: LGPL (v2)]
A  +69   -0    autotests/fake_notifications_server.h     [License: LGPL (v2)]
A  +244  -0    autotests/knotification_test.cpp     [License: LGPL (v2)]
A  +4    -0    autotests/knotifications5/qttest.notifyrc
A  +51   -0    autotests/qtest_dbus.h     [License: LGPL]
M  +9    -13   src/knotification.cpp
M  +20   -5    src/knotificationmanager.cpp
M  +2    -2    src/knotificationplugin.cpp
M  +0    -4    src/notifybypopup.cpp

http://commits.kde.org/knotifications/d4dd58759284ffe09420664e04a7a31e338e9ff1

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 51f6c22..70234c2 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -77,6 +77,7 @@ endif()
 
 add_subdirectory(src)
 add_subdirectory(tests)
+add_subdirectory(autotests)
 
 # create a Config.cmake and a ConfigVersion.cmake file and install them
 set(CMAKECONFIG_INSTALL_DIR "${KDE_INSTALL_CMAKEPACKAGEDIR}/KF5Notifications")
diff --git a/autotests/CMakeLists.txt b/autotests/CMakeLists.txt
new file mode 100644
index 0000000..17c3e3a
--- /dev/null
+++ b/autotests/CMakeLists.txt
@@ -0,0 +1,11 @@
+include(ECMAddTests)
+
+find_package(Qt5Test ${REQUIRED_QT_VERSION} CONFIG QUIET)
+
+if(NOT Qt5Test_FOUND)
+    message(STATUS "Qt5Test not found, autotests will not be built.")
+    return()
+endif()
+
+set(KNotificationTest_SRCS knotification_test.cpp fake_notifications_server.cpp)
+ecm_add_test(${KNotificationTest_SRCS} TEST_NAME "KNotificationTest" LINK_LIBRARIES \
                Qt5::Test Qt5::DBus KF5::Notifications)
diff --git a/autotests/fake_notifications_server.cpp \
b/autotests/fake_notifications_server.cpp new file mode 100644
index 0000000..4cebd6c
--- /dev/null
+++ b/autotests/fake_notifications_server.cpp
@@ -0,0 +1,79 @@
+/*
+   Copyright (C) 2016 Martin Klapetek <mklapetek@kde.org>
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Library General Public
+   License version 2 as published by the Free Software Foundation.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Library General Public License for more details.
+
+   You should have received a copy of the GNU Library General Public License
+   along with this library; see the file COPYING.LIB.  If not, write to
+   the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+   Boston, MA 02110-1301, USA.
+*/
+
+#include "fake_notifications_server.h"
+
+NotificationsServer::NotificationsServer(QObject *parent)
+{
+    Q_UNUSED(parent);
+    counter = 1;
+}
+
+uint NotificationsServer::Notify(const QString &app_name,
+                                 uint replaces_id,
+                                 const QString &app_icon,
+                                 const QString &summary,
+                                 const QString &body,
+                                 const QStringList &actions,
+                                 const QVariantMap &hints,
+                                 int timeout)
+{
+    NotificationItem i;
+    i.app_name = app_name;
+    i.replaces_id = replaces_id;
+    i.app_icon = app_icon;
+    i.summary = summary;
+    i.body = body;
+    i.actions = actions;
+    i.hints = hints;
+    i.timeout = timeout;
+    i.id = counter;
+
+    notifications.append(i);
+
+    Q_EMIT newNotification();
+
+    return counter++;
+}
+
+void NotificationsServer::CloseNotification(uint id)
+{
+    QList<NotificationItem>::iterator i = notifications.begin();
+    while (i != notifications.end()) {
+        if ((*i).id == id) {
+            notifications.erase(i);
+            break;
+        }
+        i++;
+    }
+
+    Q_EMIT NotificationClosed(id, 3);
+}
+
+QStringList NotificationsServer::GetCapabilities()
+{
+    return QStringList{QStringLiteral("body-markup"), QStringLiteral("body"), \
QStringLiteral("actions")}; +}
+
+QString NotificationsServer::GetServerInformation(QString &vendor, QString &version, \
QString &specVersion) +{
+    vendor = QLatin1String("KDE");
+    version = QLatin1String("2.0"); // FIXME
+    specVersion = QLatin1String("1.1");
+    return QStringLiteral("TestServer");
+}
diff --git a/autotests/fake_notifications_server.h \
b/autotests/fake_notifications_server.h new file mode 100644
index 0000000..dd37306
--- /dev/null
+++ b/autotests/fake_notifications_server.h
@@ -0,0 +1,69 @@
+/*
+   Copyright (C) 2016 Martin Klapetek <mklapetek@kde.org>
+
+   This library is free software; you can redistribute it and/or
+   modify it under the terms of the GNU Library General Public
+   License version 2 as published by the Free Software Foundation.
+
+   This library is distributed in the hope that it will be useful,
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+   Library General Public License for more details.
+
+   You should have received a copy of the GNU Library General Public License
+   along with this library; see the file COPYING.LIB.  If not, write to
+   the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+   Boston, MA 02110-1301, USA.
+*/
+
+#include <QObject>
+#include <QHash>
+#include <QVariantMap>
+
+class NotificationItem
+{
+public:
+    QString app_name;
+    uint replaces_id;
+    QString app_icon;
+    QString summary;
+    QString body;
+    QStringList actions;
+    QVariantMap hints;
+    int timeout;
+    uint id;
+};
+
+class NotificationsServer : public QObject
+{
+    Q_OBJECT
+    Q_CLASSINFO("D-Bus Interface", "org.freedesktop.Notifications")
+
+public:
+    NotificationsServer(QObject *parent = 0);
+
+    uint counter;
+    QList<NotificationItem> notifications;
+
+public Q_SLOTS:
+    uint Notify(const QString &app_name,
+                uint replaces_id,
+                const QString &app_icon,
+                const QString &summary,
+                const QString &body,
+                const QStringList &actions,
+                const QVariantMap &hints,
+                int timeout);
+
+    void CloseNotification(uint id);
+
+    QStringList GetCapabilities();
+
+    QString GetServerInformation(QString &vendor, QString &version, QString \
&specVersion); +
+Q_SIGNALS:
+    void NotificationClosed(uint id, uint reason);
+    void ActionInvoked(uint id, const QString &actionKey);
+
+    void newNotification();
+};
diff --git a/autotests/knotification_test.cpp b/autotests/knotification_test.cpp
new file mode 100644
index 0000000..5d063f1
--- /dev/null
+++ b/autotests/knotification_test.cpp
@@ -0,0 +1,244 @@
+/*
+    Copyright (c) 2016 Martin Klapetek <mklapetek@kde.org>
+
+    This library is free software; you can redistribute it and/or
+    modify it under the terms of the GNU Library General Public
+    License version 2 as published by the Free Software Foundation.
+
+    This library is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+    Library General Public License for more details.
+
+    You should have received a copy of the GNU Library General Public License
+    along with this library; see the file COPYING.LIB.  If not, write to
+    the Free Software Foundation, Inc., 51 Franklin Street, Fifth Floor,
+    Boston, MA 02110-1301, USA.
+*/
+
+#include <qtest.h>
+#include <QObject>
+#include <QStandardPaths>
+#include <QDBusConnection>
+
+#include "../src/knotification.h"
+#include "fake_notifications_server.h"
+#include "qtest_dbus.h"
+
+class KNotificationTest : public QObject
+{
+    Q_OBJECT
+
+private Q_SLOTS:
+    void initTestCase();
+    void cleanupTestCase();
+    void gettersTest();
+    void idTest();
+    void immediateCloseTest();
+    void serverCallTest();
+    void serverCloseTest();
+    void serverActionsTest();
+
+private:
+    NotificationsServer *m_server;
+};
+
+void KNotificationTest::initTestCase()
+{
+    QStandardPaths::setTestModeEnabled(true);
+    const QString dataDir = \
QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation); +    qDebug() \
<< dataDir; +
+    QDir dir;
+    bool pathOk = dir.mkpath(dataDir + QStringLiteral("/knotifications5/"));
+
+    QVERIFY(pathOk);
+
+    QFile testFile(QFINDTESTDATA(QStringLiteral("knotifications5/qttest.notifyrc")));
 +    bool fileOk = testFile.copy(dataDir + \
QStringLiteral("/knotifications5/qttest.notifyrc")); +
+    QVERIFY(fileOk);
+
+    m_server = new NotificationsServer(this);
+
+    bool rs = QDBusConnection::sessionBus().registerService(QStringLiteral("org.freedesktop.Notifications"));
 +    bool ro = QDBusConnection::sessionBus().registerObject(QStringLiteral("/org/freedesktop/Notifications"), \
m_server, QDBusConnection::ExportAllContents); +
+    QVERIFY(rs);
+    QVERIFY(ro);
+}
+
+void KNotificationTest::cleanupTestCase()
+{
+    // Cleanup
+    const QString dataDir = \
QStandardPaths::writableLocation(QStandardPaths::GenericDataLocation); +    QFile \
testFile(dataDir + QStringLiteral("/knotifications5/qttest.notifyrc")); +    bool \
fileRemoveOk = testFile.remove(); +
+    QVERIFY(fileRemoveOk);
+
+    QDir dir(dataDir + QStringLiteral("/knotifications5"));
+    bool dirRemoveOk = dir.removeRecursively();
+
+    QVERIFY(dirRemoveOk);
+}
+
+void KNotificationTest::gettersTest()
+{
+    const QString testEvent = QStringLiteral("testEvent");
+    const QString testText = QStringLiteral("Test");
+
+    KNotification *n = new KNotification(testEvent);
+    n->setText(QStringLiteral("Test"));
+
+    QCOMPARE(n->id(), -1);
+    QCOMPARE(n->eventId(), testEvent);
+    QCOMPARE(n->text(), testText);
+    QCOMPARE(n->title(), QString());
+    QCOMPARE(n->appName(), QCoreApplication::applicationName());
+
+    // DefaultEvent is reserved for Workspace events
+    n->setFlags(KNotification::DefaultEvent);
+    QCOMPARE(n->appName(), QStringLiteral("plasma_workspace"));
+
+    n->setFlags(KNotification::CloseOnTimeout);
+    n->setComponentName(QStringLiteral("testtest"));
+    QCOMPARE(n->appName(), QStringLiteral("testtest"));
+
+    QSignalSpy nClosedSpy(n, SIGNAL(closed()));
+    QSignalSpy nDestroyedSpy(n, SIGNAL(destroyed(QObject*)));
+
+    // Calling ref and deref simulates a Notification plugin
+    // starting and ending an action, after the action has
+    // finished, the notification should close itself
+    n->ref();
+    n->deref();
+
+    QCOMPARE(nClosedSpy.size(), 1);
+
+    // ...and delete itself too
+    nDestroyedSpy.wait(500);
+    QCOMPARE(nDestroyedSpy.size(), 1);
+}
+
+void KNotificationTest::idTest()
+{
+    KNotification n(QStringLiteral("testEvent"));
+
+    QCOMPARE(n.id(), -1);
+
+    n.sendEvent();
+
+    // The first id is 0; the previous test did not
+    // call sendEvent() and therefore should not have
+    // increased the id counter, which starts at 0
+    QCOMPARE(n.id(), 0);
+
+    // TODO
+    // At this point there is no clean/sane way
+    // to test the other id() changes throughout
+    // the life of KNotification, ideally there
+    // would be a signal sending out the changed
+    // id values that would be caught by a signal spy
+    // and then checked
+}
+
+void KNotificationTest::immediateCloseTest()
+{
+    KNotification *n = new KNotification(QStringLiteral("testEvent"));
+
+    QSignalSpy nClosedSpy(n, SIGNAL(closed()));
+
+    n->close();
+
+    nClosedSpy.wait(100);
+
+    QCOMPARE(nClosedSpy.size(), 1);
+}
+
+void KNotificationTest::serverCallTest()
+{
+    const QString testText = QStringLiteral("Test");
+
+    QSignalSpy serverNewSpy(m_server, SIGNAL(newNotification()));
+    QSignalSpy serverClosedSpy(m_server, SIGNAL(NotificationClosed(uint,uint)));
+
+    KNotification n(QStringLiteral("testEvent"));
+    n.setText(testText);
+    n.setFlags(KNotification::Persistent);
+
+    n.sendEvent();
+
+    serverNewSpy.wait(500);
+
+    QCOMPARE(m_server->notifications.last().body, testText);
+    // timeout 0 is persistent notification
+    QCOMPARE(m_server->notifications.last().timeout, 0);
+
+    QCOMPARE(n.id(), 1);
+
+    // Give the dbus communication some time to finish
+    QTest::qWait(300);
+
+    n.close();
+
+    serverClosedSpy.wait(500);
+    QCOMPARE(serverClosedSpy.size(), 1);
+    QCOMPARE(m_server->notifications.size(), 0);
+}
+
+void KNotificationTest::serverCloseTest()
+{
+    KNotification n(QStringLiteral("testEvent"));
+    n.setText(QStringLiteral("Test"));
+    n.setFlags(KNotification::Persistent);
+    n.sendEvent();
+
+    QSignalSpy nClosedSpy(&n, SIGNAL(closed()));
+
+    // Give the dbus some time
+    QTest::qWait(300);
+
+    uint id = m_server->notifications.last().id;
+
+    m_server->NotificationClosed(id, 2);
+
+    nClosedSpy.wait(100);
+
+    QCOMPARE(nClosedSpy.size(), 1);
+}
+
+void KNotificationTest::serverActionsTest()
+{
+    KNotification n(QStringLiteral("testEvent"));
+    n.setText(QStringLiteral("Test"));
+    n.setActions(QStringList{QStringLiteral("a1"), QStringLiteral("a2")});
+    n.sendEvent();
+
+    QSignalSpy serverClosedSpy(m_server, SIGNAL(NotificationClosed(uint,uint)));
+    QSignalSpy nClosedSpy(&n, SIGNAL(closed()));
+    QSignalSpy nActivatedSpy(&n, SIGNAL(activated(uint)));
+    QSignalSpy nActivated2Spy(&n, SIGNAL(action1Activated()));
+
+    QTest::qWait(300);
+
+    uint id = m_server->notifications.last().id;
+
+    Q_EMIT m_server->ActionInvoked(id, QString::number(1));
+
+    nActivatedSpy.wait(300);
+    // After the notification action was invoked,
+    // the notification should request closing
+    serverClosedSpy.wait(300);
+    nClosedSpy.wait(300);
+
+    QCOMPARE(serverClosedSpy.size(), 1);
+    QCOMPARE(nActivatedSpy.size(), 1);
+    QCOMPARE(nActivated2Spy.size(), 1);
+    // The argument must be 1, as was passed to the ActionInvoked
+    QCOMPARE(nActivatedSpy.at(0).at(0).toInt(), 1);
+    QCOMPARE(nClosedSpy.size(), 1);
+}
+
+QTEST_GUILESS_MAIN_SYSTEM_DBUS(KNotificationTest)
+#include "knotification_test.moc"
diff --git a/autotests/knotifications5/qttest.notifyrc \
b/autotests/knotifications5/qttest.notifyrc new file mode 100644
index 0000000..e16c8a8
--- /dev/null
+++ b/autotests/knotifications5/qttest.notifyrc
@@ -0,0 +1,4 @@
+[Global]
+
+[Event/testEvent]
+Action=Popup
diff --git a/autotests/qtest_dbus.h b/autotests/qtest_dbus.h
new file mode 100644
index 0000000..8ef649e
--- /dev/null
+++ b/autotests/qtest_dbus.h
@@ -0,0 +1,51 @@
+/*
+    Copyright 2014 Alejandro Fiestas Olivares <afiestas@kde.org>
+
+    This library is free software; you can redistribute it and/or
+    modify it under the terms of the GNU Lesser General Public
+    License as published by the Free Software Foundation; either
+    version 2.1 of the License, or (at your option) version 3, or any
+    later version accepted by the membership of KDE e.V. (or its
+    successor approved by the membership of KDE e.V.), which shall
+    act as a proxy defined in Section 6 of version 3 of the license.
+
+    This library is distributed in the hope that it will be useful,
+    but WITHOUT ANY WARRANTY; without even the implied warranty of
+    MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+    Lesser General Public License for more details.
+
+    You should have received a copy of the GNU Lesser General Public
+    License along with this library. If not, see <http://www.gnu.org/licenses/>.
+*/
+
+
+#ifndef KNOTIFICATIONS_QTEST_DBUS_H
+#define KNOTIFICATIONS_QTEST_DBUS_H
+
+#include <QtTest>
+#include <stdlib.h>
+
+#define QTEST_GUILESS_MAIN_SYSTEM_DBUS(TestObject) \
+int main(int argc, char *argv[]) \
+{ \
+    QProcess dbus; \
+    dbus.start(QStringLiteral("dbus-launch")); \
+    dbus.waitForFinished(10000);    \
+    QByteArray session = dbus.readLine(); \
+    if (session.isEmpty()) { \
+        qFatal("Couldn't execute new dbus session"); \
+    } \
+    int pos = session.indexOf('='); \
+    setenv("DBUS_SESSION_BUS_ADDRESS", session.right(session.count() - pos - \
1).trimmed().constData(), 1); \ +    session = dbus.readLine(); \
+    pos = session.indexOf('='); \
+    QByteArray pid = session.right(session.count() - pos - 1).trimmed(); \
+    QCoreApplication app( argc, argv ); \
+    app.setApplicationName( QLatin1String("qttest") ); \
+    TestObject tc; \
+    int result = QTest::qExec( &tc, argc, argv ); \
+    dbus.start(QStringLiteral("kill"), QStringList() << QStringLiteral("-9") << \
QString::fromLatin1(pid)); \ +    dbus.waitForFinished(); \
+    return result; \
+}
+#endif //KNOTIFICATIONS_QTEST_DBUS_H
diff --git a/src/knotification.cpp b/src/knotification.cpp
index 17b0be2..352cf49 100644
--- a/src/knotification.cpp
+++ b/src/knotification.cpp
@@ -60,7 +60,7 @@ struct KNotification::Private {
     QTimer updateTimer;
     bool needUpdate;
 
-    Private() : id(0), ref(0), widget(0l), needUpdate(false) {}
+    Private() : id(-1), ref(0), widget(0l), needUpdate(false) {}
     /**
      * recursive function that raise the widget. @p w
      *
@@ -97,7 +97,7 @@ KNotification::KNotification(
 
 KNotification::~KNotification()
 {
-    if (d->id > 0) {
+    if (d->id >= 0) {
         KNotificationManager::self()->close(d->id);
     }
     delete d;
@@ -140,7 +140,7 @@ void KNotification::setTitle(const QString &title)
 
     d->needUpdate = true;
     d->title = title;
-    if (d->id > 0) {
+    if (d->id >= 0) {
         d->updateTimer.start();
     }
 }
@@ -153,7 +153,7 @@ void KNotification::setText(const QString &text)
 
     d->needUpdate = true;
     d->text = text;
-    if (d->id > 0) {
+    if (d->id >= 0) {
         d->updateTimer.start();
     }
 }
@@ -166,7 +166,7 @@ void KNotification::setIconName(const QString &icon)
 
     d->needUpdate = true;
     d->iconName = icon;
-    if (d->id > 0) {
+    if (d->id >= 0) {
         d->updateTimer.start();
     }
 }
@@ -185,7 +185,7 @@ void KNotification::setPixmap(const QPixmap &pix)
 {
     d->needUpdate = true;
     d->pixmap = pix;
-    if (d->id > 0) {
+    if (d->id >= 0) {
         d->updateTimer.start();
     }
 }
@@ -203,7 +203,7 @@ void KNotification::setActions(const QStringList &as)
 
     d->needUpdate = true;
     d->actions = as;
-    if (d->id > 0) {
+    if (d->id >= 0) {
         d->updateTimer.start();
     }
 }
@@ -264,8 +264,6 @@ void KNotification::activate(unsigned int action)
     // which will deref() the KNotification object, which will result
     // in closing the notification
     emit activated(action);
-
-    d->id = -1;
 }
 
 void KNotification::close()
@@ -279,7 +277,6 @@ void KNotification::close()
         emit closed();
         deleteLater();
     }
-
 }
 
 void KNotification::raiseWidget()
@@ -376,7 +373,6 @@ void KNotification::ref()
 {
     d->ref++;
 }
-
 void KNotification::deref()
 {
     d->ref--;
@@ -394,9 +390,9 @@ void KNotification::beep(const QString &reason, QWidget *widget)
 void KNotification::sendEvent()
 {
     d->needUpdate = false;
-    if (d->id == 0) {
+    if (d->id == -1) {
         d->id = KNotificationManager::self()->notify(this);
-    } else if (d->id > 0) {
+    } else if (d->id >= 0) {
         KNotificationManager::self()->reemit(this);
     }
 }
diff --git a/src/knotificationmanager.cpp b/src/knotificationmanager.cpp
index e80d48c..6f790b5 100644
--- a/src/knotificationmanager.cpp
+++ b/src/knotificationmanager.cpp
@@ -165,11 +165,26 @@ void KNotificationManager::notificationClosed()
 void KNotificationManager::close(int id, bool force)
 {
     if (force || d->notifications.contains(id)) {
-        KNotification *n = d->notifications.take(id);
+        KNotification *n = d->notifications.value(id);
         qCDebug(LOG_KNOTIFICATIONS) << "Closing notification" << id;
 
-        Q_FOREACH (KNotificationPlugin *plugin, d->notifyPlugins) {
-            plugin->close(n);
+        // Find plugins that are actually acting on this notification
+        // call close() only on those, otherwise each KNotificationPlugin::close()
+        // will call finish() which may close-and-delete the KNotification object
+        // before it finishes calling close on all the other plugins.
+        // For example: Action=Popup is a single actions but there is 5 loaded
+        // plugins, calling close() on the second would already close-and-delete
+        // the notification
+        KNotifyConfig notifyConfig(n->appName(), n->contexts(), n->eventId());
+        QString notifyActions = notifyConfig.readEntry(QStringLiteral("Action"));
+
+        Q_FOREACH (const QString &action, notifyActions.split('|')) {
+            if (!d->notifyPlugins.contains(action)) {
+                qCDebug(LOG_KNOTIFICATIONS) << "No plugin for action" << action;
+                continue;
+            }
+
+            d->notifyPlugins[action]->close(n);
         }
     }
 }
@@ -191,7 +206,7 @@ int KNotificationManager::notify(KNotification *n)
         return -1;
     }
 
-    d->notifications.insert(++d->notifyIdCounter, n);
+    d->notifications.insert(d->notifyIdCounter, n);
 
     Q_FOREACH (const QString &action, notifyActions.split('|')) {
         if (!d->notifyPlugins.contains(action)) {
@@ -206,7 +221,7 @@ int KNotificationManager::notify(KNotification *n)
     }
 
     connect(n, &KNotification::closed, this, \
                &KNotificationManager::notificationClosed);
-    return d->notifyIdCounter;
+    return d->notifyIdCounter++;
 }
 
 void KNotificationManager::update(KNotification *n)
diff --git a/src/knotificationplugin.cpp b/src/knotificationplugin.cpp
index acf964c..8598f7b 100644
--- a/src/knotificationplugin.cpp
+++ b/src/knotificationplugin.cpp
@@ -42,11 +42,11 @@ void KNotificationPlugin::update(KNotification *notification, \
KNotifyConfig *con  
 void KNotificationPlugin::close(KNotification *notification)
 {
-	emit finished(notification);
+    emit finished(notification);
 }
 
 void KNotificationPlugin::finish(KNotification *notification)
 {
-	emit finished(notification);
+    emit finished(notification);
 }
 
diff --git a/src/notifybypopup.cpp b/src/notifybypopup.cpp
index c0050b2..7d69a36 100644
--- a/src/notifybypopup.cpp
+++ b/src/notifybypopup.cpp
@@ -471,9 +471,6 @@ void NotifyByPopup::onGalagoNotificationActionInvoked(uint \
notificationId, const  KNotification *n = *iter;
     if (n) {
         emit actionInvoked(n->id(), actionKey.toUInt());
-        // now close notification - similar to popup behaviour
-        // (popups are hidden after link activation - see 'connects' of \
                linkActivated signal above)
-        d->closeGalagoNotification(n);
     } else {
         d->galagoNotifications.erase(iter);
     }
@@ -743,7 +740,6 @@ void NotifyByPopupPrivate::closeGalagoNotification(KNotification \
*notification)  if (!queued) {
         qCWarning(LOG_KNOTIFICATIONS) << "Failed to queue dbus message for closing a \
notification";  }
-
 }
 
 void NotifyByPopupPrivate::queryPopupServerCapabilities()


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic