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

Permalinks to threads are difficult for clients to render #150

Open
clokep opened this issue Feb 17, 2022 · 1 comment
Open

Permalinks to threads are difficult for clients to render #150

clokep opened this issue Feb 17, 2022 · 1 comment

Comments

@clokep
Copy link
Contributor

clokep commented Feb 17, 2022

The /context API gets used to render the room timeline when clicking on a permalink to a message (since the client may not have all the surrounding events). Due to the /context API returning non-sense for events in a thread (see below) it is difficult to properly render a portion of a thread when clicking on a permalink to a thread.

The workaround is to fetch the entire thread using the /relations API, which could be expensive for a large thread and involve many round-trips, which is a poor user experience on mobile.


Technical bits

Calling /context on a threaded event returns other events around it in the topological order of the room, which is not very useful.

For example if you have the following events in a room where D is a threaded reply to A:

flowchart RL;
	F-->E;
	E-->D;
	D-->C;
	C-->B;
	B-->A;
	D-->|`m.thread` relation|A;
	F-->|`m.thread` relation|A;	
Loading

Calling /rooms/!roomId/context/D?limit=1 would return something like (note that this is simplified since we just care about the events):

{
	"event": "D",
	"events_before": ["C"],
    "events_after": ["E"]
}

This is nonsensical since D has nothing to do with C or E, it would likely make more sense to include A as the "event before" (as the root event) and perhaps F as "events after" as another event relating to A.

In the contrived example above you can get around it by increasing the limit, but this is not a reasonable workaround in a busy room.

I think this probably applies to any relation, not just threads, but is "worse" for threads since you likely wouldn't call /context on e.g. an annotation.

As a related piece of work, the above also applies to any location where we fetch "context" for an event. Other locations include:


Potential work breakdown:

  1. Write an MSC to describe the change in behavior.
  2. Implement the MSC in Synapse.
  3. Implement the MSC in clients.

Background

It was briefly discussed in MSC2675 whether the /context API should be changed for relations. See the screenshot below because that PR seems to break GitHub:

image

@clokep
Copy link
Contributor Author

clokep commented Feb 22, 2022

Note that the workaround for this isn't as dire as mentioned in the above, essentially if the event returned includes a relation, then a second request to /relations can be made on the root event. Hopefully the thread is short and pagination is then not needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant