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

MSC4037: Thread root is not in the thread #4037

Closed
wants to merge 18 commits into from
Closed
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
110 changes: 110 additions & 0 deletions proposals/4037-thread-root-is-not-in-thread.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
# MSC4037: Thread root is not in the thread

The current spec implies that a thread root is considered within the thread, but
we argue that this does not make sense, and a thread root is not "in" the thread
branching from it.
andybalaam marked this conversation as resolved.
Show resolved Hide resolved

This is important for creating and interpreting read receipts.

## Motivation

The current spec, in
[11.6.2.2 Threaded read receipts](https://spec.matrix.org/v1.7/client-server-api/#threaded-read-receipts)
says:

> An event is considered to be "in a thread" if it meets any of the following
> criteria:
>
> * It has a `rel_type` of `m.thread`.
> * It has child events with a `rel_type` of `m.thread` (in which case it’d be
> the thread root).
> * Following the event relationships, it has a parent event which qualifies for
> one of the above. Implementations should not recurse infinitely, though: a
> maximum of 3 hops is recommended to cover indirect relationships.
>
> Events not in a thread but still in the room are considered to be part of the
> "main timeline", or a special thread with an ID of `main`.

This explicitly includes thread roots in the thread which branches off them, and
implicitly _excludes_ those messages from being in the `main` thread.
andybalaam marked this conversation as resolved.
Show resolved Hide resolved

This is problematic because:

* It seems natural for messages that are displayed in the main timeline (as
thread roots are in most clients) to be considered read/unread when the user
reads them in the main timeline.

* It normally does not make sense for a threaded read receipt to point at the
thread root, since the user has not really read anything in that thread if
they have only read the thread root.

In practice, Synapse
[ignores any request to mark the thread root as read](https://github.com/matrix-org/synapse/blob/v1.87.0/synapse/rest/client/receipts.py#L116-L154)
within the thread, and accepts requests to mark it as read in the main timeline.

In consequence, Element Web exhibited bugs relating to unread rooms while its
underlying library used spec-compliant behaviour, many of which were fixed by
[adopting the behaviour recommended by this proposal](https://github.com/matrix-org/matrix-js-sdk/pull/3600).

It really does not make sense to treat thread roots as outside the main
timeline: any message can become a thread root at any time, when a user creates
a new threaded message pointing at it, so suddenly switching which receipts are
allowed to apply to it would not be sensible.

Similarly, it does not make sense for reactions to the thread root (or other
related events such as edits) to be outside the main timeline, for similar
reasons: the message we are reacting to can become a thread root at any time,
making our previous receipt invalid retrospectively. (We could conceivably allow
receipts to exist both within a thread and the main timeline[^1], but this does not
match the expected user mental model: I have either read a reaction/edit/reply,
or I have not - I don't want to have to read it twice just because it appears in
two places in the UI.)

[^1]: In fact, observation of Synapse's behaviour shows that it does appear to
track two read/unread statuses for edits of thread roots, but not for thread
roots themselves. The code for this is in
[receipts.py](https://github.com/matrix-org/synapse/blob/v1.87.0/synapse/rest/client/receipts.py#L116-L154).
We still argue that this behaviour does not match the user's mental model.

## Proposal
andybalaam marked this conversation as resolved.
Show resolved Hide resolved

We propose that thread roots are in the main timeline, making the definition:

> An event is considered to be "in a thread" if:
>
> * It has a `rel_type` of `m.thread`, or it has an ancestor event with this
andybalaam marked this conversation as resolved.
Show resolved Hide resolved
> `rel_type`.
>
> Implementations should limit recursion to find ancestors: a maximum of 3 hops
> is recommended.
>
> Events not in a thread but still in the room are considered to be part of the
> "main timeline": a special thread with an ID of `main`.
>
> Note: thread roots (events that are referred to in a `m.thread` relationship)
> are in the main timeline. Similarly, reactions to thread roots, edits of
> thread root, and other events with non-thread relations to a thread root are
> in the main timline.
andybalaam marked this conversation as resolved.
Show resolved Hide resolved

## Potential issues

None known.

## Alternatives

We could treat thread roots as being in *both* their thread and the `main`
timeline, but it does not offer much benefit because a thread where only the
root message has been read is almost identical to one where the no messages have
been read. A thread cannot exist without at least one additional message.

## Security considerations

Unlikely to have any security impact.

## Unstable prefix

None needed.

## Dependencies

No dependencies.
Loading