Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Always clear unread status in room and send read receipt when using "Mark as read" feature #25611

Closed
MadLittleMods opened this issue Jun 15, 2023 · 3 comments
Labels
A-Notifications O-Occasional Affects or can be seen by some users regularly or most users rarely T-Enhancement X-Cannot-Reproduce

Comments

@MadLittleMods
Copy link
Contributor

MadLittleMods commented Jun 15, 2023

Your use case

What would you like to do?

Always clear unread status in room and send read receipt when using "Mark as read" feature

Why would you like to do it?

The "Mark as read" features are not reliable and can often have no effect leaving your room still as unread.

(whether pressing escape or using the "Mark as unread" option)

How would you like to achieve it?

Make the clearRoomNotification(...) logic robust:

  • Always choose the latest event
    • The logic around the latest event is obviously flawed because it uses event timestamps (untrusted and there so many race conditions where your perfectly accurate timestamp event can be behind or in front of another). It needs to use the latest event according to the DAG
    • lastThread has the same timestamp based flaws
    • Do we really not have access to a unthreaded timeline where we can just grab the latest event from /sync and /messages?
  • If it's a decision between two events, just send receipts for both anyway
  • Even if the app already believes it sent a receipt and is caught up, just send it again
  • Add feedback when an error occurs
  • Clearing notification counts as a last ditch effort without sending the receipts to back that up is just a recipe to have the unreads appear after refresh.

Have you considered any alternatives?

No response

Additional context

No response

@robintown robintown added the O-Occasional Affects or can be seen by some users regularly or most users rarely label Jun 19, 2023
@t3chguy
Copy link
Member

t3chguy commented Jun 19, 2023

Even if the app already believes it sent a receipt and is caught up, just send it again

As far as I can see it sends a read receipt unconditionally already..?

image

I just pressed the button a few times and it sent the same RR.

image

Do we really not have access to a unthreaded timeline where we can just grab the latest event from /sync and /messages?

No as we ideally would not be paginating (/messages) threaded events in the same event stream as the main timeline: matrix-org/matrix-spec-proposals#3874 - trying to maintain a single total order timeline would require forgoing any such thing which means scrolling up in a room is slow if the room is thread-heavy, as the client will just be loading walls of thread events it cannot render in the main timeline.

The logic around the latest event is obviously flawed because it uses event timestamps (untrusted and there so many race conditions where your perfectly accurate timestamp event can be behind or in front of another). It needs to use the latest event according to the DAG

That's tracked in matrix-org/matrix-js-sdk#3325

@MadLittleMods
Copy link
Contributor Author

@t3chguy The point of this issue is for "Mark as read" to actually do what it says. "Mark as read" from the context menu sends read receipts but it doesn't actually accomplish the job for a variety of reasons:

(room still unread after clicking "Mark as read")

If we look at the keyboard shortcut Esc, it doesn't do anything if you're scrolled to the bottom already. And there is actually a caveat that it only clears notifications in the main timeline so the room still remains unread. Both random details that don't help when you're just trying to clear unreads in the room.

@MadLittleMods MadLittleMods changed the title Always send read receipt when using "Mark as read" feature Always clear unread status in room and send read receipt when using "Mark as read" feature Jun 19, 2023
@t3chguy
Copy link
Member

t3chguy commented Jun 20, 2023

Duplicate of #24229

@t3chguy t3chguy marked this as a duplicate of #24229 Jun 20, 2023
@t3chguy t3chguy closed this as completed Jun 20, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jun 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Notifications O-Occasional Affects or can be seen by some users regularly or most users rarely T-Enhancement X-Cannot-Reproduce
Projects
None yet
Development

No branches or pull requests

3 participants