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.

Sign up for GitHub

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

Jump to bottom

Stuck unread in thread where last message is a reaction #26718

Open
dbkr opened this issue Dec 8, 2023 · 0 comments
Open

Stuck unread in thread where last message is a reaction #26718

dbkr opened this issue Dec 8, 2023 · 0 comments
Labels
A-Notifications O-Occasional Affects or can be seen by some users regularly or most users rarely S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect

Comments

@dbkr
Copy link
Member

dbkr commented Dec 8, 2023 •
edited
Loading

Steps to reproduce

Writing up a bug for my discoveries on this case of stuck unreads. This is most likely the cause of at last one other stuck unread bug in the tracker, but rather than co-opt one of them I'm just going to make a dedicated issue.

I haven't actually tested these repro steps yet as I've ben analysing an existing thread, but hypothetically:

  1. Make a thread
  2. Post some messages in it
  3. Post a reaction to the last message
  4. In a different client, open that thread so the unread dot goes away
  5. Refresh that new client

The unread dot has probably come back (and the reaction has maybe disappeared?) This is because your read receipt points to the reaction event which didn't get sent with the thread relations because it's an indirect relation. Unless your server has MSC3981 support enabled, in which case perhaps it all worked fine?

Unknowns:

  • Does the reaction definitely disappear and if so, why do I see reactions working fine in other threads that have loaded post-refresh?
  • Is it all fine with MSC3981 support?
    • Tried a server with it enabled and it still isn't returning the related events??
  • How could we fix this for servers without MSC3981 support? (I'm short on ideas...)
  • How are edits seemingly working but reactions are not?

Outcome

What did you expect?

What happened instead?

Operating system

No response

Application version

No response

How did you install the app?

No response

Homeserver

No response

Will you send logs?

No
@dbkr dbkr added the T-Defect label Dec 8, 2023
dbkr added a commit to matrix-org/matrix-js-sdk that referenced this issue Dec 12, 2023
@dbkr
Remove m.thread filter from relations API call
c0de03e 
We used MSC3981 to pass the recurse param to the /relations
endpoint so that we could get relations to events in a thread, but
we kept the rel_type filter on (as m.thread) so no second-order relations
would ever have been returned (a nested thread isn't a thing).

This removes the filter and does some filtering on the client side to
remove any events that shouldn't live in the threaded timeline (ie.
non-thread relations to the thread root event).

This should help fix stuck unreads because it will avoid the event that
the receipt refers to going missing (but only on HSes that support MSC3981).

For element-hq/element-web#26718
@dbkr dbkr mentioned this issue Dec 12, 2023
Merged
3 tasks
@MidhunSureshR MidhunSureshR added S-Major Severely degrades major functionality or product features, with no satisfactory workaround A-Notifications O-Occasional Affects or can be seen by some users regularly or most users rarely labels Dec 13, 2023
github-merge-queue bot pushed a commit to matrix-org/matrix-js-sdk that referenced this issue Dec 13, 2023
@dbkr
Remove m.thread filter from relations API call (#3959)
e35a820 
* Remove m.thread filter from relations API call

We used MSC3981 to pass the recurse param to the /relations
endpoint so that we could get relations to events in a thread, but
we kept the rel_type filter on (as m.thread) so no second-order relations
would ever have been returned (a nested thread isn't a thing).

This removes the filter and does some filtering on the client side to
remove any events that shouldn't live in the threaded timeline (ie.
non-thread relations to the thread root event).

This should help fix stuck unreads because it will avoid the event that
the receipt refers to going missing (but only on HSes that support MSC3981).

For element-hq/element-web#26718

* Fix import cycle

* Remove params from expected calls in tests to match

* Unused import
github-merge-queue bot pushed a commit to matrix-org/matrix-js-sdk that referenced this issue Dec 13, 2023
@dbkr
Remove m.thread filter from relations API call (#3959)
b9f13bd 
* Remove m.thread filter from relations API call

We used MSC3981 to pass the recurse param to the /relations
endpoint so that we could get relations to events in a thread, but
we kept the rel_type filter on (as m.thread) so no second-order relations
would ever have been returned (a nested thread isn't a thing).

This removes the filter and does some filtering on the client side to
remove any events that shouldn't live in the threaded timeline (ie.
non-thread relations to the thread root event).

This should help fix stuck unreads because it will avoid the event that
the receipt refers to going missing (but only on HSes that support MSC3981).

For element-hq/element-web#26718

* Fix import cycle

* Remove params from expected calls in tests to match

* Unused import
github-merge-queue bot pushed a commit to matrix-org/matrix-js-sdk that referenced this issue Dec 14, 2023
@dbkr
Remove m.thread filter from relations API call (#3959)
5dfc30f 
* Remove m.thread filter from relations API call

We used MSC3981 to pass the recurse param to the /relations
endpoint so that we could get relations to events in a thread, but
we kept the rel_type filter on (as m.thread) so no second-order relations
would ever have been returned (a nested thread isn't a thing).

This removes the filter and does some filtering on the client side to
remove any events that shouldn't live in the threaded timeline (ie.
non-thread relations to the thread root event).

This should help fix stuck unreads because it will avoid the event that
the receipt refers to going missing (but only on HSes that support MSC3981).

For element-hq/element-web#26718

* Fix import cycle

* Remove params from expected calls in tests to match

* Unused import
github-merge-queue bot pushed a commit to matrix-org/matrix-js-sdk that referenced this issue Dec 14, 2023
@dbkr
Remove m.thread filter from relations API call (#3959)
f604ab2 
* Remove m.thread filter from relations API call

We used MSC3981 to pass the recurse param to the /relations
endpoint so that we could get relations to events in a thread, but
we kept the rel_type filter on (as m.thread) so no second-order relations
would ever have been returned (a nested thread isn't a thing).

This removes the filter and does some filtering on the client side to
remove any events that shouldn't live in the threaded timeline (ie.
non-thread relations to the thread root event).

This should help fix stuck unreads because it will avoid the event that
the receipt refers to going missing (but only on HSes that support MSC3981).

For element-hq/element-web#26718

* Fix import cycle

* Remove params from expected calls in tests to match

* Unused import
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-Notifications O-Occasional Affects or can be seen by some users regularly or most users rarely S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@dbkr @MidhunSureshR