"Unread messages" banner stays when scrolled to end

[gnprice]

Pretty often, when I'm catching up on a conversation and the "N unread messages" banner is showing, I'll scroll through it to the end... and then the banner is still there. Usually with just a handful of messages unread.

Then if I scroll slightly back up from the end, the banner goes away.

Now that #3125 is merged, this should get substantially easier to investigate.

[GonzRon]

I think I will file a ticket for this if it's not already reported:
I am using the android mobile client, friend is using IOS, we are both experiencing the same behavior.
Two behaviors:

1. This message ("unread messages") appears for a topic / stream even when it is you that has entered the message,
2. the "mark all messages as read" button that subsequently appears seems to do nothing.

[gnprice]

Hi @GonzRon ! Please do file issues for those -- those reports will be helpful. Let's keep this issue thread focused on the specific issue described in the description at the top.

[gnprice]

I just reproduced this on v25.6.120, the latest beta:

• Went to #test here on czo -- a stream I subscribe to, have muted, and always have tons of unreads in.
• Went to the list of topics.
• Picked a topic that had a larger number of unreads -- 15 unreads -- and navigated there.
• That message list opened. The banner read "6 unread messages". After a second or so, the green "unread" highlights faded from all the messages in view.
• I tried moving around a bit, and I was at the end -- it wasn't possible to scroll down.

So, probably the unreads were older than the messages on the screen? That seems like a clue.

[jainkuniya]

There are two possibilities for this issue:

• Unread count in the banner comes from the unread state. The only way it is updated is via server events. So if user scrolls, we send messageFlags to server and server again sends messageFlags event and we update it in the store. So if there is internet connectivity issue (slow) then there is high possibility that there will be no effect on unread count in banner even after scrolling multiple times.
  I have reproduced it when my mobile data is slow.
  Solution: Something in the direction of Fire EVENT_UPDATE_MESSAGE_FLAGS on reading message. #1422. So on message scroll, update unread count state locally as well (and send messageFlag update to server). So that we don't wait until messages are really marked as read on server. And our api calls will update messageFlags in background (current behaviour).

• Another issue is in our queueMarkAsRead.js

if (Date.now() - lastSentTime > 2000) {
    console.log('Date.now() - lastSentTime > 2000', unsentMessageIds);
    messagesFlags(auth, unsentMessageIds, 'add', 'read');
    unsentMessageIds = [];
    lastSentTime = Date.now();
  }

As soon as user scrolls message list, we emit scroll events with messageIds from webview and pass them to this queue. Now imagine some events came and this queue called messagesFlags(auth, unsentMessageIds, 'add', 'read');. Now assume that next event came in less than 2000 ms (2s), this queue will end up adding those messageIds in the queue and will not fire a API call as Date.now() - lastSentTime > 2000 is false (this is basically to throttle api calls, because scroll events are frequent and we don't want to bombard server). So until user scroll again our logic won't be calling any API.

Example: markMessages got called, then there are more scroll events which got stuck as they came before 2s.

• On Android, user have to scroll up and down, because there us no bounce scroll on Android unlike iOS.
• On iOS, scroll events are too frequent, which ends up calling queue and clearing queue too frequent.

Solution: Calling of messagesFlags should not depend on call of queueMarkAsRead. Work of queueMarkAsRead should be just add messages to the queue. And some other job/thread/function should periodically check for messages in the queue and call messagesFlags.

[gnprice]

Thanks @jainkuniya for the investigation and the detailed writeup!

I agree that both of those points are problems. I just filed #3549 for the first one -- which also aligns with a bug report Rishi made just last week. I'm happy that #3125 is helping us sort out so many of these issues that have been there for a long time 😃

I think the second one, about queueMarkAsRead, is probably the main cause of the symptoms reported in this thread -- so let's focus the rest of this thread on that.

A few months ago we looked at the same function in the context of a different problem, and I filed #3423 describing several issues with it. But it looks like I didn't notice this one!

We discussed this by video chat just now. I agree with your diagnosis, and with the general direction to fix it. Specifically what I'd like us to do is:

• Let's use setTimeout in queueMarkAsRead, with a callback to go ahead and send.
• Let's keep the timer in a variable. If there's already a timer, we don't need to set a new one. When the callback fires, we'll have it clear the timer variable.

The corresponding webapp code is in static/js/message_flags.js inside send_read. You may find that helpful to compare to.