don't show read messages in chat notifications
This patch prevent showing already read messages in chat notifications
which was happening in certain cases. This is fixed in several
different ways depending on which notification daemon is being used
on the system.
In the case of notify-osd, even though it supports appending
notifications, we try to update the previous notification, but only
with unread messages. The issue with appending is that notify-osd
does not respond to trying to close notifications, which means messages
which have been marked as read already will continue to be displayed.
In the case of the notification daemons which don't support appending,
we simply replace the old msg text. This prevents many notifications
from the same person from building up; the new messages are also
displayed immediately instead of waiting for the notification timeout.
We also don't try to display multiple unread messages because these
daemons don't usually support multi-line message bodies.
Change-Id: Ibbd5adbdd5eb4bafadb517ac39064eaecd74228e
Tuleap: #426
diff --git a/src/ringnotify.cpp b/src/ringnotify.cpp
index 8499ac5..65aea79 100644
--- a/src/ringnotify.cpp
+++ b/src/ringnotify.cpp
@@ -35,11 +35,66 @@
#include <media/recordingmodel.h>
#endif
+#if USE_LIBNOTIFY
+
+static constexpr int MAX_NOTIFICATIONS = 10; // max unread chat msgs to display from the same contact
+static constexpr const char* SERVER_NOTIFY_OSD = "notify-osd";
+
+/* struct to store the parsed list of the notify server capabilities */
+struct RingNotifyServerInfo
+{
+ /* info */
+ char *name;
+ char *vendor;
+ char *version;
+ char *spec;
+
+ /* capabilities */
+ gboolean append;
+ gboolean actions;
+
+ /* the info strings must be freed */
+ ~RingNotifyServerInfo() {
+ g_free(name);
+ g_free(vendor);
+ g_free(version);
+ g_free(spec);
+ }
+};
+
+static struct RingNotifyServerInfo server_info;
+#endif
+
void
ring_notify_init()
{
#if USE_LIBNOTIFY
notify_init("Ring");
+
+ /* get notify server info */
+ if (notify_get_server_info(&server_info.name,
+ &server_info.vendor,
+ &server_info.version,
+ &server_info.spec)) {
+ g_debug("notify server name: %s, vendor: %s, version: %s, spec: %s",
+ server_info.name, server_info.vendor, server_info.version, server_info.spec);
+ }
+
+ /* check notify server capabilities */
+ auto list = notify_get_server_caps();
+ while (list) {
+ if (g_strcmp0((const char *)list->data, "append") == 0 ||
+ g_strcmp0((const char *)list->data, "x-canonical-append") == 0) {
+ server_info.append = TRUE;
+ }
+ if (g_strcmp0((const char *)list->data, "actions") == 0) {
+ server_info.actions = TRUE;
+ }
+
+ list = g_list_next(list);
+ }
+
+ g_list_free_full(list, g_free);
#endif
}
@@ -125,14 +180,35 @@
#if USE_LIBNOTIFY
+static void
+ring_notify_free_list(gpointer, GList *value, gpointer)
+{
+ if (value) {
+ g_object_unref(G_OBJECT(value->data));
+ g_list_free(value);
+ }
+}
+
+static void
+ring_notify_free_chat_table(GHashTable *table) {
+ if (table) {
+ g_hash_table_foreach(table, (GHFunc)ring_notify_free_list, nullptr);
+ g_hash_table_destroy(table);
+ }
+}
+
+/**
+ * Returns a pointer to a GHashTable which contains key,value pairs where a ContactMethod pointer
+ * is the key and a GList of notifications for that CM is the vlue.
+ */
GHashTable *
ring_notify_get_chat_table()
{
- static std::unique_ptr<GHashTable, decltype(g_hash_table_destroy)&> chat_table(
- nullptr, g_hash_table_destroy);
+ static std::unique_ptr<GHashTable, decltype(ring_notify_free_chat_table)&> chat_table(
+ nullptr, ring_notify_free_chat_table);
if (chat_table.get() == nullptr)
- chat_table.reset(g_hash_table_new_full(NULL, NULL, NULL, g_object_unref));
+ chat_table.reset(g_hash_table_new(NULL, NULL));
return chat_table.get();
}
@@ -142,12 +218,19 @@
{
g_return_if_fail(cm);
- if (!g_hash_table_remove(ring_notify_get_chat_table(), cm)) {
- g_warning("could not find notification associated with the given ContactMethod");
- /* normally removing the notification from the hash table will unref it,
- * but if it was not found we should do it here */
- g_object_unref(notification);
+ /* remove from the list */
+ auto chat_table = ring_notify_get_chat_table();
+ if (auto list = (GList *)g_hash_table_lookup(chat_table, cm)) {
+ list = g_list_remove(list, notification);
+ if (list) {
+ // the head of the list may have changed
+ g_hash_table_replace(chat_table, cm, list);
+ } else {
+ g_hash_table_remove(chat_table, cm);
+ }
}
+
+ g_object_unref(notification);
}
static gboolean
@@ -156,58 +239,89 @@
g_return_val_if_fail(idx.isValid() && cm, FALSE);
gboolean success = FALSE;
- GHashTable *chat_table = ring_notify_get_chat_table();
+ auto title = g_markup_printf_escaped(C_("Text message notification", "%s says:"), idx.data(static_cast<int>(Ring::Role::Name)).toString().toUtf8().constData());
+ auto body = g_markup_escape_text(idx.data(Qt::DisplayRole).toString().toUtf8().constData(), -1);
- auto title = g_strdup_printf(C_("Text message notification", "%s says:"), idx.data(static_cast<int>(Ring::Role::Name)).toString().toUtf8().constData());
- auto body = g_strdup_printf("%s", idx.data(Qt::DisplayRole).toString().toUtf8().constData());
+ NotifyNotification *notification_new = nullptr;
+ NotifyNotification *notification_old = nullptr;
- /* check if a notification already exists for this CM */
- NotifyNotification *notification = (NotifyNotification *)g_hash_table_lookup(chat_table, cm);
- if (notification) {
- /* update notification; append the new message to the old */
- GValue body_value = G_VALUE_INIT;
- g_value_init(&body_value, G_TYPE_STRING);
- g_object_get_property(G_OBJECT(notification), "body", &body_value);
- const gchar* body_old = g_value_get_string(&body_value);
- if (body_old && (strlen(body_old) > 0)) {
- gchar *body_new = g_strconcat(body_old, "\n", body, NULL);
- g_free(body);
- body = body_new;
+ /* try to get the previous notification */
+ auto chat_table = ring_notify_get_chat_table();
+ auto list = (GList *)g_hash_table_lookup(chat_table, cm);
+ if (list)
+ notification_old = (NotifyNotification *)list->data;
+
+ /* we display chat notifications in different ways to suit different notification servers and
+ * their capabilities:
+ * 1. if the server doesn't support appending (eg: Notification Daemon) then we update the
+ * previous notification (if exists) with new text; otherwise it takes we have many
+ * notifications from the same person... we don't concatinate the old messages because
+ * servers which don't support append usually don't support multi line bodies
+ * 2. the notify-osd server supports appending; however it doesn't clear the old notifications
+ * on demand, which means in our case that chat messages which have already been read could
+ * still be displayed when a new notification is appended, thus in this case, we update
+ * the old notification body manually to only contain the unread messages
+ * 3. the 3rd case is that the server supports append but is not notify-osd, then we simply use
+ * the append feature
+ */
+
+ if (notification_old && !server_info.append) {
+ /* case 1 */
+ notify_notification_update(notification_old, title, body, nullptr);
+ notification_new = notification_old;
+ } else if (notification_old && g_strcmp0(server_info.name, SERVER_NOTIFY_OSD) == 0) {
+ /* case 2 */
+ /* print up to MAX_NOTIFICATIONS unread messages */
+ int msg_count = 0;
+ auto idx_next = idx.sibling(idx.row() - 1, idx.column());
+ auto read = idx_next.data(static_cast<int>(Media::TextRecording::Role::IsRead)).toBool();
+ while (idx_next.isValid() && !read && msg_count < MAX_NOTIFICATIONS) {
+
+ auto body_prev = body;
+ body = g_markup_printf_escaped("%s\n%s", body_prev, idx_next.data(Qt::DisplayRole).toString().toUtf8().constData());
+ g_free(body_prev);
+
+ idx_next = idx_next.sibling(idx_next.row() - 1, idx_next.column());
+ read = idx_next.data(static_cast<int>(Media::TextRecording::Role::IsRead)).toBool();
+ ++msg_count;
}
- notify_notification_update(notification, title, body, NULL);
+
+ notify_notification_update(notification_old, title, body, nullptr);
+
+ notification_new = notification_old;
} else {
- /* create new notification object and associate it with the CM in the
- * hash table; also store the pointer of the CM in the notification
- * object so that it knows it's key in the hash table */
- notification = notify_notification_new(title, body, NULL);
- g_hash_table_insert(chat_table, cm, notification);
- g_object_set_data(G_OBJECT(notification), "ContactMethod", cm);
+ /* need new notification for case 1, 2, or 3 */
+ notification_new = notify_notification_new(title, body, nullptr);
+
+ /* track in hash table */
+ auto list = (GList *)g_hash_table_lookup(chat_table, cm);
+ list = g_list_append(list, notification_new);
+ g_hash_table_replace(chat_table, cm, list);
/* get photo */
QVariant var_p = GlobalInstances::pixmapManipulator().callPhoto(
cm, QSize(50, 50), false);
std::shared_ptr<GdkPixbuf> photo = var_p.value<std::shared_ptr<GdkPixbuf>>();
- notify_notification_set_image_from_pixbuf(notification, photo.get());
+ notify_notification_set_image_from_pixbuf(notification_new, photo.get());
/* normal priority for messages */
- notify_notification_set_urgency(notification, NOTIFY_URGENCY_NORMAL);
+ notify_notification_set_urgency(notification_new, NOTIFY_URGENCY_NORMAL);
/* remove the key and value from the hash table once the notification is
* closed; note that this will also unref the notification */
- g_signal_connect(notification, "closed", G_CALLBACK(notification_closed), cm);
+ g_signal_connect(notification_new, "closed", G_CALLBACK(notification_closed), cm);
+ }
+
+ GError *error = nullptr;
+ success = notify_notification_show(notification_new, &error);
+ if (!success) {
+ g_warning("failed to show notification: %s", error->message);
+ g_clear_error(&error);
}
g_free(title);
g_free(body);
- GError *error = NULL;
- success = notify_notification_show(notification, &error);
- if (!success) {
- g_warning("failed to show notification: %s", error->message);
- g_clear_error(&error);
- g_hash_table_remove(chat_table, cm);
- }
-
return success;
}
@@ -294,24 +408,23 @@
g_return_val_if_fail(cm, FALSE);
- GHashTable *chat_table = ring_notify_get_chat_table();
+ auto chat_table = ring_notify_get_chat_table();
- NotifyNotification *notification = (NotifyNotification *)g_hash_table_lookup(chat_table, cm);
+ if (auto list = (GList *)g_hash_table_lookup(chat_table, cm)) {
+ while (list) {
+ notification_existed = TRUE;
+ auto notification = (NotifyNotification *)list->data;
- if (notification) {
- notification_existed = TRUE;
+ GError *error = NULL;
+ if (!notify_notification_close(notification, &error)) {
+ g_warning("could not close notification: %s", error->message);
+ g_clear_error(&error);
+ }
- GError *error = NULL;
- if (!notify_notification_close(notification, &error)) {
- g_warning("could not close notification: %s", error->message);
- g_clear_error(&error);
-
- /* closing should remove and free the notification from the hash table
- * since it failed to close, try to remove the notification from the
- * table manually */
- g_hash_table_remove(chat_table, cm);
+ list = g_list_next(list);
}
}
+
#endif
return notification_existed;