Fix a bunch of cases where the read marker wasn't being updated - #3513
Fix a bunch of cases where the read marker wasn't being updated#3513ruka-hamanasu wants to merge 3 commits into
Conversation
|
I'm going to try to figure out the minimal set of changes that fixes the problems, since I think a couple of these are too aggressive and could mark rooms as read when there are genuinely unread messages in them. |
|
Ok, I've made it less aggressive about updating the read marker, so it shouldn't mark events are read that the user hasn't actually seen or at least scrolled past. |
|
@ruka-hamanasu can you please tell me how to reproduce the original problem on main branch? Because every time I mark a room as read on main branch it works perfectly fine so I thought the problem was already fixed |
the repro is: on any Continuwuity or Tuwunel homeserver, use FluffyChat in rooms that are set up as "mentions only", wait for dark blue dot to appear, read room, observe dark blue dot never goes away. DMed you a reg token for my homeserver which has a couple additional fixes for cases where Tuwunel's behavior exacerbated the problems, but they cover 1 niche case (room with only backfilled events), a case where if someone reacts to something while you're reading the room won't be marked read, and an interaction with other clients where the read marker can move backwards. This fix should show improvement on any Tuwunel or Continuwuity server because it makes FluffyChat advance the read marker even if there are only non-notifying events (messages but no mentions) in the room. Not sure why same doesn't happen with Synapse honestly (or even that it doesn't since I don't have a Synapse account). |
|
Due to a report by a user who only uses FluffyChat that this wasn't happening to them, I investigated possible interactions with other clients. I've confirmed that the problem only occurs if you are also using a client that doesn't post On the bright side, that means you don't need a Tuwunel server to reproduce this; that was a red herring. All you need to do is to log Cinny into your account, read a few unread rooms with Cinny, then go and try to read those same rooms from FluffyChat. |
setReadMarker() had a chain of failure modes that each left rooms silently unmarked forever: - The receipt target was gated on push rule evaluation, so rulesets that notify for nothing (e.g. mentions-only) could never mark any room read. - The scroll-up banner suppressed marking even when already at the bottom; check the live scroll position instead of the cached _scrolledUp flag, which could get stuck when no scroll transition ever fired. - The "nothing new" skip ignored whether the marker had reached the target event, so a marker left behind by another device never moved. - A failed request left _setReadMarkerFuture set, permanently blocking all later attempts in that room; reset it in whenComplete and log failures.
17b3fab to
53de5e1
Compare
These cases would cause the dark blue dot to remain on a room (non-notifying events):
m.readorm.private_readbut notm.fully_read.Tested by doing all the stuff CI does locally on my machine and by running an APK build on my phone. My homeserver/account are highly prone to this issue and I had several stuck rooms previously.
This interacts with a couple of Tuwunel bugs that I have also put up PRs for: matrix-construct/tuwunel#564 and matrix-construct/tuwunel#565 . Also now matrix-construct/tuwunel#566 which only impacts rooms with only backfilled events (like when the user is the first joiner from the local homeserver).
I am happy to split out the Cinny interaction fix into a separate PR if you like; that's the most impactful one for people who use multiple clients.
Pull Request has been tested on: