zulip-flutter icon indicating copy to clipboard operation
zulip-flutter copied to clipboard

Avoid duplicate message flag on redundant event

Open gnprice opened this issue 2 weeks ago • 0 comments

I observed an odd symptom today on my phone, which @chrisbobbe and I debugged far enough to know how to fix.

The symptom: there was a message which the app stubbornly showed as read (no blue bar in the message list). I could long-press and hit "mark unread" all I wanted, and the message would show as unread when I checked separately in web, but it kept being shown as read in the app.

Initially, the Unreads data structure agreed in showing the message as read. (As evidenced by the blue "Mark messages as read" button being absent at the bottom of the message list, and by there being no unread badge for the topic when I visited the channel's topic-list page.) Then I marked the message as read again (by first marking the previous message as unread, then using the blue mark-read button), then unread again (from the message's action sheet)… and the message continued being shown as read (no blue bar along the side), but now the Unreads data structure correctly showed it as unread.

Diagnosis

One bug here is in the logic for handling a mark-read event:

        isAdd
          ? message.flags.add(event.flag)
          : message.flags.remove(event.flag);

The message.flags field is a List. So if the .read flag is already present, and then gets added again, there'll be two of them. Then if it gets removed, List.remove will only remove one copy, and the message will still be seen as read.

To fix this, we should avoid adding a flag if it's already present in the list. That's a condition that can happen, because of the fetch-event race.

Further

Even after we fix that, I think that won't address what caused the initial situation:

Initially, the Unreads data structure agreed in showing the message as read.

Given that the server apparently saw the message as unread at that point, and that the situation was stable, that suggests that we'd somehow dropped entirely a previous event that marked the message unread. I don't currently have a good hypothesis for that part.

gnprice avatar Nov 12 '25 23:11 gnprice