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

New reaction to old event ends up on the main timeline instead of the thread #25450

Closed
Tracked by #24392
andybalaam opened this issue May 26, 2023 · 1 comment · Fixed by matrix-org/matrix-js-sdk#3427
Closed
Tracked by #24392

New reaction to old event ends up on the main timeline instead of the thread #25450

andybalaam opened this issue May 26, 2023 · 1 comment · Fixed by matrix-org/matrix-js-sdk#3427
Assignees
@t3chguy
Labels
O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect X-Release-Blocker

Comments

@andybalaam
Copy link
Contributor

Steps to reproduce

  1. Turn on seeing hidden events in devtools
  2. In a room, create a thread with some messages on it
  3. Add at least 20 new messages to the main timeline
  4. React to the last message in the thread with some emoji
  5. Refresh the page
  6. Look for where the reaction event is

Outcome

What did you expect?

I expected my reaction to be shown after the last message in the thread

What happened instead?

My reaction is invisible, and if I look for the hidden event for it, it's been added on the main timeline instead of in the thread

It's because of the logic in matrix-jsd-sdk/src/models/room.ts in eventShouldLiveIn. If we can't find the parent event of a relation, we give up and say it should go into the main timeline.

Maybe we should put it in a holding area and go and fetch the parent event, so we can decide what thread the reaction lives in.

Operating system

Ubuntu 22.04

Browser information

Firefox 112.0.2

URL for webapp

https://develop.element.io

Application version

Element version: 6b7f71f-react-530197bfcd5c-js-b29e1e9d21b5 Olm version: 3.2.14

Homeserver

matrix.org

Will you send logs?

No
@andybalaam andybalaam changed the title Reaction ends up on the main timeline instead of the thread New reaction to old event ends up on the main timeline instead of the thread May 26, 2023
@andybalaam andybalaam mentioned this issue May 26, 2023
Open
@andybalaam
Copy link
Contributor Author

Linking this to #24392 because it causes some stuck notifications.

@germain-gg germain-gg mentioned this issue May 31, 2023
Open
@t3chguy t3chguy mentioned this issue May 31, 2023
Merged
@weeman1337 weeman1337 added S-Major Severely degrades major functionality or product features, with no satisfactory workaround O-Uncommon Most users are unlikely to come across this or unexpected workflow labels May 31, 2023
@Johennes Johennes mentioned this issue Jun 2, 2023
Open
su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this issue Dec 13, 2023
@su-ex
Merge tag 'v26.0.0' into sc
54d9823 
* Ensure we do not add relations to the wrong timeline ([\matrix-org#3427](matrix-org#3427)). Fixes element-hq/element-web#25450 and element-hq/element-web#25494.
* Deprecate `QrCodeEvent`, `SasEvent` and `VerificationEvent` ([\matrix-org#3386](matrix-org#3386)).
* Move crypto classes into a separate namespace ([\matrix-org#3385](matrix-org#3385)).
* Mention deno support in the README ([\matrix-org#3417](matrix-org#3417)). Contributed by @sigmaSd.
* Mark room version 10 as safe ([\matrix-org#3425](matrix-org#3425)).
* Prioritise entirely supported flows for UIA ([\matrix-org#3402](matrix-org#3402)).
* Add methods to terminate idb worker ([\matrix-org#3362](matrix-org#3362)).
* Total summary count ([\matrix-org#3351](matrix-org#3351)). Contributed by @toger5.
* Audio concealment ([\matrix-org#3349](matrix-org#3349)). Contributed by @toger5.
* Correctly accumulate sync summaries. ([\matrix-org#3366](matrix-org#3366)). Fixes element-hq/element-web#23345.
* Keep measuring a call feed's volume after a stream replacement ([\matrix-org#3361](matrix-org#3361)). Fixes element-hq/element-call#1051.
* Element-R: Avoid uploading a new fallback key at every `/sync` ([\matrix-org#3338](matrix-org#3338)). Fixes element-hq/element-web#25215.
* Accumulate receipts for the main thread and unthreaded separately ([\matrix-org#3339](matrix-org#3339)). Fixes element-hq/element-web#24629.
* Remove spec non-compliant extended glob format ([\matrix-org#3423](matrix-org#3423)). Fixes element-hq/element-web#25474.
* Fix bug where original event was inserted into timeline instead of the edit event ([\matrix-org#3398](matrix-org#3398)). Contributed by @andybalaam.
* Only add a local receipt if it's after an existing receipt ([\matrix-org#3399](matrix-org#3399)). Contributed by @andybalaam.
* Attempt a potential workaround for stuck notifs ([\matrix-org#3384](matrix-org#3384)). Fixes element-hq/element-web#25406. Contributed by @andybalaam.
* Fix verification bug with `pendingEventOrdering: "chronological"` ([\matrix-org#3382](matrix-org#3382)).
@matrixbot matrixbot mentioned this issue Dec 22, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@t3chguy t3chguy

Labels
O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect X-Release-Blocker
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Ensure we do not add relations to the wrong timeline matrix-org/matrix-js-sdk
4 participants
@andybalaam @Johennes @t3chguy @weeman1337