Fix notification not being cleared, when read event didn't cause a notification

2 files changed, 19 insertions, 36 deletions

diff --git a/src/notifications/Manager.h b/src/notifications/Manager.h
index 6cbecbc6..e6be5953 100644
--- a/src/notifications/Manager.h
+++ b/src/notifications/Manager.h
@@ -1,6 +1,5 @@
 #pragma once
 
-#include <QHash>
 #include <QImage>
 #include <QObject>
 #include <QString>
@@ -17,26 +16,11 @@ struct roomEventId
 };
 
 inline bool
-operator<(const roomEventId &a, const roomEventId &b)
-{
-        if (a.roomId == b.roomId)
-                return a.eventId < b.eventId;
-        else
-                return a.roomId < b.roomId;
-}
-
-inline bool
 operator==(const roomEventId &a, const roomEventId &b)
 {
         return a.roomId == b.roomId && a.eventId == b.eventId;
 }
 
-inline uint
-qHash(const roomEventId &v, uint seed)
-{
-        return qHash(v.roomId, seed) ^ qHash(v.eventId, seed);
-}
-
 class NotificationsManager : public QObject
 {
         Q_OBJECT
@@ -67,7 +51,6 @@ private:
 
         // notification ID to (room ID, event ID)
         QMap<uint, roomEventId> notificationIds;
-        QHash<roomEventId, uint> eventToNotificationId;
 #endif
 
         // these slots are platform specific (D-Bus only)
diff --git a/src/notifications/ManagerLinux.cpp b/src/notifications/ManagerLinux.cpp
index 33fcb41d..8b7b41bb 100644
--- a/src/notifications/ManagerLinux.cpp
+++ b/src/notifications/ManagerLinux.cpp
@@ -40,7 +40,6 @@ NotificationsManager::postNotification(const QString &roomid,
 {
         uint id             = showNotification(roomname, sender + ": " + text, icon);
         notificationIds[id] = roomEventId{roomid, eventid};
-        eventToNotificationId[roomEventId{roomid, eventid}] = id;
 }
 /**
  * This function is based on code from
@@ -98,23 +97,24 @@ NotificationsManager::closeNotification(uint id)
 
 void
 NotificationsManager::removeNotification(const QString &roomId, const QString &eventId)
-{
-        roomEventId reId = {roomId, eventId};
-        if (eventToNotificationId.contains(reId)) {
-                for (auto elem = notificationIds.begin(); elem != notificationIds.end(); ++elem) {
-                        if (elem.value().roomId != roomId)
-                                continue;
-
-                        // close all notifications matching the eventId or having a lower
-                        // notificationId
-                        // This relies on the notificationId not wrapping around. This allows for
-                        // approximately 2,147,483,647 notifications, so it is a bit unlikely.
-                        // Otherwise we would need to store a 64bit counter instead.
-                        closeNotification(elem.key());
-
-                        if (elem.value() == reId)
-                                break;
-                }
+
+  roomEventId reId = {roomId, eventId};
+for (auto elem = notificationIds.begin(); elem != notificationIds.end(); ++elem) {
+        if (elem.value().roomId != roomId)
+                continue;
+
+        // close all notifications matching the eventId or having a lower
+        // notificationId
+        // This relies on the notificationId not wrapping around. This allows for
+        // approximately 2,147,483,647 notifications, so it is a bit unlikely.
+        // Otherwise we would need to store a 64bit counter instead.
+        closeNotification(elem.key());
+
+        // FIXME: compare index of event id of the read receipt and the notification instead of just
+        // the id to prevent read receipts of events without notification clearing all notifications
+        // in that room!
+        if (elem.value() == reId)
+                break;
         }
 }
 
@@ -131,7 +131,7 @@ void
 NotificationsManager::notificationClosed(uint id, uint reason)
 {
         Q_UNUSED(reason);
-        eventToNotificationId.remove(notificationIds.take(id));
+        notificationIds.remove(id);
 }
 
 /**