MSC4103: Make threaded read receipts opt-in in /sync#4103
MSC4103: Make threaded read receipts opt-in in /sync#4103
Conversation
turt2live
added
client-server
There was a problem hiding this comment.
Implementation requirements:
- Client using the opt-in flag to enable the behaviour.
- Client not using the opt-in flag which clearly fixes the bug described.
| them `m.read` events as received by the server. If this filter is missing or false, then the server must only send the | ||
| client `m.read` events whose `thread_id` is missing or `main`. |
There was a problem hiding this comment.
I think this is an API breaking change, because one would expect to receive threaded receipts from /sync by default. So, to preserve backwards compatibility with clients, how about making it opt-out? That is, thread receipts would be sent if the filter is true or missing, and not sent if it's set to false.
There was a problem hiding this comment.
..if we're having to add a filter to filter them out for thread unaware clients then why not just ask clients to check for thread_id: main? I think Matthew's point is that it was a breaking change to include threaded RRs by default, and this is trying to fix it. This puts the onus on threaded clients to add this filter flag to ask for threaded receipts. I agree it is a breaking change from the current status quo though.
There was a problem hiding this comment.
I think this MSC also implies a server-side change: now a server can't merge a threaded receipt with an unthreaded receipt on the same event id anymore, because two clients might request the two different flavors of the filter. Am I understanding it right?
There was a problem hiding this comment.
I don't see this anywhere in the MSC. The proposal is just:
- add
threaded_read_receiptsas a sync filter param. - default to ??? if missing (default=true doesn't break threaded clients today, default=false breaks threaded clients but unbreaks thread unaware clients which were broken when MSC3771 landed.
- if set, send ALL THE RECEIPTS. If unset, only send unthreaded/thread_id:main receipts.
Co-authored-by: Benjamin Bouvier <[email protected]>
| @@ -0,0 +1,77 @@ | |||
| # MSC4103: Make threaded read receipts opt-in in /sync | |||
There was a problem hiding this comment.
| We could solve this by making threaded RRs a different event type (e.g. `m.read_thread`), so non-thread-aware clients | ||
| would automatically ignore them. However, this would require threaded clients to send double the read receipts whenever | ||
| the user reads the main thread - both an `m.read` and an `m.read_thread`, which feels inefficient. (It would however | ||
| mean that [MSC4102](https://github.com/matrix-org/matrix-spec-proposals/pull/4102) would not be necessary, as the RR types | ||
| would be clearly distinct). |
There was a problem hiding this comment.
This was rejected by MSC3771 FWIW (see https://github.com/matrix-org/matrix-spec-proposals/blob/main/proposals/3771-read-receipts-for-threads.md#receipt-type). Not saying it shouldn't be revisited, just noting this.
| This means that non-thread-aware clients are not spammed or confused with irrelevant threaded read receipts, and can | ||
| calculate read state purely based on main timeline activity. | ||
|
|
||
| ## Alternatives |
There was a problem hiding this comment.
Unsure it is even worth considering, but MSC3773 did add the unread_thread_notifications flag to filters which essentially means "I understand threads". It could be re-used, but that feels a bit awful. Is there a situation where a client might realistically set one and not the other?
5 participants
Rendered
Disclaimer: Written with my SCT hat on. (Other hats include SCT lead, Matrix project lead, Matrix guardian, CEO/CTO at Element)