Created attachment 175417 [details] [review]
Implement Sent signals for Telepathy.

This will allow us to update our notifications when
someone uses a different Telepathy client to send
messages.

*** Bug 635990 has been marked as a duplicate of this bug. ***

Created attachment 175478 [details] [review]
Implement Sent signals for Telepathy.

This will allow us to update our notifications when
someone uses a different Telepathy client to send
messages.

Rebased to master after overview-layout merge.

Comment on attachment 175478 [details] [review]
Implement Sent signals for Telepathy.

>Subject: [PATCH] Implement Sent signals for Telepathy.

Make that:

    telepathyClient: Implement Sent signals

>+        this._sentMessages = [];

Needs a better name. "_sentMessages" implies that it includes *all* sent messages.

It might be simpler to just not bother with that, and create this._notification at construct time, rename _ensureNotification() to _ensureSourceInMessageTray() or something, and then have _messageSent() always appendMessage()

>+    _messageSent: function(channel, timestamp, type, text) {

It's worth adding a comment before this pointing out explicitly that this is called both for messages we send ourselves, and for messages sent from other clients.

>+            this._notification.appendMessage(text, timestamp, true);
>+            this.notify(this._notification);

I don't think we want to notify() in the Sent case.

>+    appendMessage: function(text, timestamp, sent) {

don't use a boolean. No one will remember which way means "true" and which means "false". Use an enum type, or just pass in the style name directly.

Created attachment 175488 [details] [review]
telepathyClient: Implement Sent signals.

This will allow us to update our notifications when
someone uses a different Telepathy client to send
messages.

Created attachment 175491 [details] [review]
telepathyClient: Implement Sent signals.

This will allow us to update our notifications when
someone uses a different Telepathy client to send
messages.

Created attachment 175499 [details] [review]
telepathyClient: Implement Sent signals.

This will allow us to update our notifications when
someone uses a different Telepathy client to send
messages.

Created attachment 175500 [details] [review]
telepathyClient: Implement Sent signals.

This will allow us to update our notifications when
someone uses a different Telepathy client to send
messages.



Sorry about the bug-spam people. I've been trying to wrangle git-bz
into working for me.

Comment on attachment 175500 [details] [review]
telepathyClient: Implement Sent signals.

>+// Not currently referenced by the search API, but
>+// this enumeration can be useful for provider
>+// implementations.
>+const NotificationDirection = {

I think you accidentally copied that comment from somewhere else?

>+    _messageSent: function(channel, timestamp, type, text) {

Still think this should get a comment

>+    appendMessage: function(text, timestamp, direction) {
>+        let styles = {};
>+        styles[NotificationDirection.SENT] = 'chat-sent';
>+        styles[NotificationDirection.RECEIVED] = 'chat-received';

You don't want to recompute this every time appendMessage() is called. You could either define it at the top level, or else just change the enum so that the value of NotificationDirection.SENT is 'chat-sent' rather than 0, etc.


I see what you mean about Source.notify... it's sort of a mess. Some classes redefine it, but no one ever calls .notify on a source of an unknown type, so it always ends up working out. We might want to fix it up somehow at some point, but it doesn't have to be part of this patch.

Created attachment 175560 [details] [review]
telepathyClient: Implement Sent signals.

This will allow us to update our notifications when
someone uses a different Telepathy client to send
messages.

Created attachment 175571 [details] [review]
telepathyClient: Implement Sent signals.

This will allow us to update our notifications when
someone uses a different Telepathy client to send
messages.

Attachment 175571 [details] pushed as b9eca84 - telepathyClient: Implement Sent signals.