matrix-org / matrix-org/matrix-js-sdk

Relation event insertion breaks during backwards pagination if parent is on another page (if `supportsThreads: true`)

Open
#3,937 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
TypeScript
Stars
2.2k
Forks
704
Avg merge
1d 20h
Merged PRs (30d)
40

Description

Assume that we have a `PARENT` event a `CHILD` event which is an `m.replace` of `PARENT`, sent a long time after `PARENT` was sent (so there are some pages between the two events).

If I have `supportsThreads: true`, and I'm backfilling the room via `paginateEventTimeline` (going in backwards direction), I hit `CHILD` first, going through `addEventsToTimeline` -> `eventShouldLiveIn`, leading me to...

https://github.com/matrix-org/matrix-js-sdk/blob/b515cdbdbbe21d481409c677af365a5315fb6390/src/models/room.ts#L2171

Since I didn't hit `PARENT` yet (and it is not in the page that is currently being processed, i.e. `events`), `parentEvent` is not found, causing the following case to fail (I think this case is the one that *should* be hit, although we don't have parent so we can't recur):

https://github.com/matrix-org/matrix-js-sdk/blob/b515cdbdbbe21d481409c677af365a5315fb6390/src/models/room.ts#L2174-L2177

All other branches fail, causing the function to return:

https://github.com/matrix-org/matrix-js-sdk/blob/b515cdbdbbe21d481409c677af365a5315fb6390/src/models/room.ts#L2198-L2204

This omits `CHILD` from the timeline (and I think shows `PARENT` instead?).

(I'm probably going to just disable `supportsThreads` to workaround this for myself.)

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start in src/models/room.ts at paginateEventTimeline, addEventsToTimeline, and eventShouldLiveIn, especially the relation handling around the linked lines. Reproduce backwards pagination with supportsThreads: true when a child m.replace event appears before its parent on a later page. Done means the child is retained in the timeline instead of being omitted when its parent is not in the current page.

Written by the indexing model from the issue text.

Assessment

Tech stack
typescript
Domain
backend
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.