Skip to content

telepathyClient: Use GObjects based message objects

Marco Trevisan requested to merge 3v1n0/gnome-shell:telepathy-fixes into master

As per commit b5676a2a ChatNotification is a GObject, but I was wrongly considering that it was using Tp.Message's as children, instead it just uses custom-built objects to pass information around through signals.

Given gjs can only use GObjects as signal parameters, create a small wrapper class to hold the ChatNotification messages and use it as signal parameter.

Fixes #2449 (closed)

!1112 (merged)


Another option to fix this without other gobjects could be instead


commit 10c1d756d7056036379237a3a860f38d2b0db601
Author: Marco Trevisan (Treviño) <mail@3v1n0.net>
Date:   Fri Mar 20 18:42:26 2020 +0100

    telepathyClient: Pass message IDs instead of objects

diff --git a/js/ui/components/telepathyClient.js b/js/ui/components/telepathyClient.js
index e6ae340424..d28657307d 100644
--- a/js/ui/components/telepathyClient.js
+++ b/js/ui/components/telepathyClient.js
@@ -632,9 +632,9 @@ class ChatSource extends MessageTray.Source {
 
 var ChatNotification = HAVE_TP ? GObject.registerClass({
     Signals: {
-        'message-removed': { param_types: [Tp.Message.$gtype] },
-        'message-added': { param_types: [Tp.Message.$gtype] },
-        'timestamp-changed': { param_types: [Tp.Message.$gtype] },
+        'message-removed': { param_types: [GObject.TYPE_UINT] },
+        'message-added': { param_types: [GObject.TYPE_UINT] },
+        'timestamp-changed': { param_types: [GObject.TYPE_UINT] },
     },
 }, class ChatNotification extends MessageTray.Notification {
     _init(source) {
@@ -644,6 +644,8 @@ var ChatNotification = HAVE_TP ? GObject.registerClass({
         this.setResident(true);
 
         this.messages = [];
+        this.messagesMap = new Map();
+        this._messageId = 0;
         this._timestampTimeoutId = 0;
     }
 
@@ -714,7 +716,7 @@ var ChatNotification = HAVE_TP ? GObject.registerClass({
             let lastMessageToKeep = filteredHistory[maxLength];
             let expired = this.messages.splice(this.messages.indexOf(lastMessageToKeep));
             for (let i = 0; i < expired.length; i++)
-                this.emit('message-removed', expired[i]);
+                this.emit('message-removed', expired[i].id);
         }
     }
 
@@ -741,13 +743,16 @@ var ChatNotification = HAVE_TP ? GObject.registerClass({
             GLib.source_remove(this._timestampTimeoutId);
         this._timestampTimeoutId = 0;
 
-        let message = { realMessage: props.group != 'meta',
-                        showTimestamp: false };
+        let message = {
+            id: this._messageId++,
+            realMessage: props.group != 'meta',
+            showTimestamp: false,
+        };
         Lang.copyProperties(props, message);
         delete message.noTimestamp;
 
         this.messages.unshift(message);
-        this.emit('message-added', message);
+        this.emit('message-added', message.id);
 
         if (!props.noTimestamp) {
             let timestamp = props.timestamp;
@@ -771,7 +776,7 @@ var ChatNotification = HAVE_TP ? GObject.registerClass({
         this._timestampTimeoutId = 0;
 
         this.messages[0].showTimestamp = true;
-        this.emit('timestamp-changed', this.messages[0]);
+        this.emit('timestamp-changed', this.messages[0].id);
 
         this._filterMessages();
 
@@ -852,18 +857,19 @@ class ChatNotificationBanner extends MessageTray.NotificationBanner {
         this._messageActors = new Map();
 
         this._messageAddedId = this.notification.connect('message-added',
-            (n, message) => {
-                this._addMessage(message);
+            (n, id) => {
+                this._addMessage(this.notification.messagesMap.get(id));
             });
         this._messageRemovedId = this.notification.connect('message-removed',
-            (n, message) => {
-                let actor = this._messageActors.get(message);
-                if (this._messageActors.delete(message))
+            (n, id) => {
+                if (this._messageActors.has(id)) {
+                    let actor = this._messageActors.get(id);
                     actor.destroy();
+                }
             });
         this._timestampChangedId = this.notification.connect('timestamp-changed',
-            (n, message) => {
-                this._updateTimestamp(message);
+            (n, id) => {
+                this._updateTimestamp(this.notification.messagesMap.get(id));
             });
 
         for (let i = this.notification.messages.length - 1; i >= 0; i--)
@@ -905,13 +911,13 @@ class ChatNotificationBanner extends MessageTray.NotificationBanner {
         let lineBox = new ChatLineBox();
         lineBox.add(body);
         this._contentArea.add_actor(lineBox);
-        this._messageActors.set(message, lineBox);
+        this._messageActors.set(message.id, lineBox);
 
         this._updateTimestamp(message);
     }
 
     _updateTimestamp(message) {
-        let actor = this._messageActors.get(message);
+        let actor = this._messageActors.get(message.id);
         if (!actor)
             return;
 

Merge request reports