After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 770888 - telepathyClient: Always clear pending messages on destroy
telepathyClient: Always clear pending messages on destroy
Status: RESOLVED FIXED
Product: gnome-shell
Classification: Core
Component: general
unspecified
Other All
: Normal normal
: ---
Assigned To: gnome-shell-maint
gnome-shell-maint
Depends on:
Blocks:
 
 
Reported: 2016-09-05 10:56 UTC by Florian Müllner
Modified: 2016-09-10 17:04 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
telepathyClient: Always clear pending messages on destroy (1.64 KB, patch)
2016-09-05 10:56 UTC, Florian Müllner
committed Details | Review

Description Florian Müllner 2016-09-05 10:56:31 UTC
See patch. I'm still divided on just ignoring whether or not we are handling the channel and ack'ing messages unconditionally on destroy, but for now I went with the formally more correct approach of not touching channels we aren't handling ...
Comment 1 Florian Müllner 2016-09-05 10:56:38 UTC
Created attachment 334783 [details] [review]
telepathyClient: Always clear pending messages on destroy

Since commit 82950ecea, we acknowledge pending messages when closing a
chat notification for a channel we are handling to prevent the channel
from popping up again immediately. While this isn't an issue for channels
we don't handle, the unread messages of the destroyed notification are
still considered for the messages indicator in the top bar, which is
clearly confusing (in particular when we end up showing the indicator
without any notifications in the list). As it's arguably correct to not
meddle with a channel handled by someone else, just reset the cache of
pending messages to address this issue.
Comment 2 Rui Matos 2016-09-09 10:51:18 UTC
Review of attachment 334783 [details] [review]:

I'm not familiar with this code but, why do we queue messages from channels we don't handle into this._pendingMessages in the first place?

Anyway, this looks fine if you want to push as is
Comment 3 Florian Müllner 2016-09-09 11:24:31 UTC
(In reply to Rui Matos from comment #2)
> I'm not familiar with this code but, why do we queue messages from channels
> we don't handle into this._pendingMessages in the first place?

In this case, "handling" the channel means "owning" it. In practice, we handle the channel (at least initially) when someone starts a private chat with you, and whatever client you are using handles the channel when you start a private chat with someone. That's not a user-visible concept though, we still show the same type of notification in both cases.

(There are also channels we don't "handle" in the common sense of the word, that is we ignore them - we don't queue messages at all for those)
Comment 4 Rui Matos 2016-09-09 12:48:48 UTC
Ok, then it does indeed make sense to clear the queue when the notification is destroyed.
Comment 5 Florian Müllner 2016-09-10 17:04:45 UTC
Hmmm, somehow git-bz didn't work yesterday ...

Attachment 334783 [details] pushed as 06d0e7d - telepathyClient: Always clear pending messages on destroy