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 712217 - Message counter should not increase on currently selected channel/direct chat
Message counter should not increase on currently selected channel/direct chat
Status: RESOLVED OBSOLETE
Product: polari
Classification: Applications
Component: general
unspecified
Other Linux
: Normal normal
: ---
Assigned To: Polari maintainers
Polari maintainers
Depends on:
Blocks:
 
 
Reported: 2013-11-13 14:00 UTC by Jakub Steiner
Modified: 2021-06-10 11:05 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
roomList: Hide counter on active channel (1.86 KB, patch)
2013-11-19 18:10 UTC, Carlos Soriano
none Details | Review
roomList: Hide counter on active channel (1.90 KB, patch)
2013-11-20 12:30 UTC, Carlos Soriano
none Details | Review
chatView: Fix condition for scrolling pending messages on screen (1.75 KB, patch)
2013-11-28 18:50 UTC, Florian Müllner
committed Details | Review

Description Jakub Steiner 2013-11-13 14:00:19 UTC
When I am talking to someone (/query) the sidebar continues to show indication of missed messages with the person I am in the conversation with.
Comment 1 Carlos Soriano 2013-11-19 18:09:23 UTC
Not sure this is what we/jakub wants.
The next patch hide the counter label in the current active channel.

So messages that we didn't saw yet, still counts as a unread messages (i.e. when the scroll is above the misread messages) and the counter label will show up again when the channel becomes unfocused.
Comment 2 Carlos Soriano 2013-11-19 18:10:49 UTC
Created attachment 260272 [details] [review]
roomList: Hide counter on active channel

Hide the counter label on the current active channel
since it is supossed the user is following the current
conversation.
Comment 3 Carlos Soriano 2013-11-20 12:30:52 UTC
Created attachment 260300 [details] [review]
roomList: Hide counter on active channel

Hide the counter label on the current active channel
since it is supossed the user is following the current
conversation.
Comment 4 Florian Müllner 2013-11-20 17:38:23 UTC
(In reply to comment #3)
> Hide the counter label on the current active channel
> since it is supossed the user is following the current
> conversation.

I don't think this assumption is necessarily true - when I come back after a while (lunch etc.), I might scroll up to read up on missed conversations. If I'm mentioned while doing so, I need an indication of it or else I'm gonna miss it.

So I'd rather make sure that messages are correctly acknowledged (and possibly use a small timeout for the counter, so that it doesn't flash briefly for messages that we acknowledge almost instantly).
Comment 5 Carlos Soriano 2013-11-20 17:52:38 UTC
(In reply to comment #4)
> (In reply to comment #3)
> > Hide the counter label on the current active channel
> > since it is supossed the user is following the current
> > conversation.
> 
> I don't think this assumption is necessarily true - when I come back after a
> while (lunch etc.), I might scroll up to read up on missed conversations. If
> I'm mentioned while doing so, I need an indication of it or else I'm gonna miss
> it.
> 
> So I'd rather make sure that messages are correctly acknowledged (and possibly
Not sure what "messages are correctly acknowledged" means. Let's talk about this in IRC when available =)
> use a small timeout for the counter, so that it doesn't flash briefly for
> messages that we acknowledge almost instantly).
Sounds reasonable for the flash issue.
Comment 6 Jean-François Fortin Tam 2013-11-21 13:12:05 UTC
Just throwing some ideas out there: maybe scrolling back to the position of such messages and waiting long enough (a timer, to ensure the user was not just "scrolling past" instead of reading) and/or typing and sending a message could clear the notification counter.
Comment 7 Florian Müllner 2013-11-28 18:50:57 UTC
Created attachment 263055 [details] [review]
chatView: Fix condition for scrolling pending messages on screen

Currently the first pending message is scrolled on screen for
inactive rooms only, to avoid interfering with backlog reading.
This is a rather poor solution though, as auto-scrolling is the
desired behavior when not reading backlog. To fix, change the
condition to test for the current scroll position instead, as we
already do for non-highlighted messages.


Meh, hidden in plain sight. There might be other bugs here of course, but this definitively should take care of a couple of cases ...
Comment 8 Carlos Soriano 2013-11-29 12:16:53 UTC
Review of attachment 263055 [details] [review]:

works fine here.
Is the flashing issue another one different then?
Comment 9 Carlos Soriano 2013-11-29 12:16:55 UTC
Review of attachment 263055 [details] [review]:

works fine here.
Is the flashing issue another one different then?
Comment 10 Florian Müllner 2013-11-29 13:14:42 UTC
(In reply to comment #8)
> Is the flashing issue another one different then?

Yes. Both the room list and the chat log monitor the channel for new messages (TpTextChannel::message-received). In response, the room list updates the counter, and the chat log adds the message to the text view. Adding the message to the text view triggers the GtkAdjustment::changed signal, in response to that we check whether we should auto-scroll to the bottom - if we do that, the highlighted messages becomes visible and is marked as read (non-highlighted messages may be marked as read directly after receiving them, but those are not relevant for the counter anyway).

If the message were marked as read directly after receiving, there would be a quick flash if the room list handler is run before the chat log one. However as the chat log will return control to the main loop first, the flashing is pretty much guaranteed to happen (unless there are very serious redraw issues).

This still needs fixing, but as mentioned in comment #4, just using a small timeout for updating the counter should do the trick.

Also nekohayo's suggestion of marking pending messages as read when *sending* a message is worth considering IMHO - at least to me it makes sense :-)
Comment 11 Florian Müllner 2014-02-21 11:54:35 UTC
Comment on attachment 263055 [details] [review]
chatView: Fix condition for scrolling pending messages on screen

Attachment 263055 [details] pushed as 4b551a5 - chatView: Fix condition for scrolling pending messages on screen
Comment 12 André Klapper 2021-06-10 11:05:52 UTC
GNOME is going to shut down bugzilla.gnome.org in favor of gitlab.gnome.org.
As part of that, we are mass-closing older open tickets in bugzilla.gnome.org
which have not seen updates for a longer time (resources are unfortunately
quite limited so not every ticket can get handled).

If you can still reproduce the situation described in this ticket in a recent
and supported software version of Polari, then please follow
  https://wiki.gnome.org/GettingInTouch/BugReportingGuidelines
and create a new ticket at
  https://gitlab.gnome.org/GNOME/polari/-/issues/

Thank you for your understanding and your help.