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

Stuck notification in thread #23921

Closed
robintown opened this issue Dec 6, 2022 · 5 comments · Fixed by matrix-org/matrix-js-sdk#3002
Closed

Stuck notification in thread #23921

robintown opened this issue Dec 6, 2022 · 5 comments · Fixed by matrix-org/matrix-js-sdk#3002
Assignees
@germain-gg
Labels
A-Message-Editing A-Notifications A-Threads O-Occasional Affects or can be seen by some users regularly or most users rarely S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect

Comments

@robintown
Copy link
Member

Steps to reproduce

Unclear, but this is what happened:

  1. Go offline
  2. Have someone start a thread in a room where you receive notifications, and then edit their threaded message
  3. Go online
  4. Open the thread

Outcome

What did you expect?

The notification count should go down from 2 to 0

What happened instead?

The notification count went down from 2 to 1, and the thread remained marked as unread in the thread list. Restarting the client was necessary to clear the notification.

Operating system

NixOS unstable

Browser information

Firefox 107.0.1

URL for webapp

develop.element.io

Application version

Element version: c473cb6-react-9914b0bafd23-js-ccab6985ad55 Olm version: 3.2.12

Homeserver

Synapse 1.72.0

Will you send logs?

No
@robintown robintown added T-Defect S-Major Severely degrades major functionality or product features, with no satisfactory workaround A-Notifications A-Message-Editing O-Uncommon Most users are unlikely to come across this or unexpected workflow A-Threads labels Dec 6, 2022
@germain-gg germain-gg mentioned this issue Dec 8, 2022
Merged
3 tasks
@gaelgatelement
Copy link
Member

I reproduced this issue in the Delight room.

The room shield appears as red.

The notification counter appeared as grey. Selecting the thread did not reset the counter.

When talking into the room, it seems that the notification disappeared.

@germain-gg germain-gg mentioned this issue Dec 9, 2022
Merged
3 tasks
@t3chguy t3chguy added O-Occasional Affects or can be seen by some users regularly or most users rarely and removed O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Dec 20, 2022
@t3chguy
Copy link
Member

t3chguy commented Dec 20, 2022

One cause of this is when the edit event is the latest thing in the unread thread, I wonder if the load-thread-from-server API doesn't include the edit event and thus it not being in the timeline results in the RR being set on the wrong event (the original event ID rather than the ID of the edit event)

image

I have show hidden events enabled.

Given how frequent edits are, this will be happening whenever the latest event in a thread is an edit. And entirely destroys productivity by wedging an unread dot on the space, the room and the thread in the room list.

image
image
image

@t3chguy
Copy link
Member

t3chguy commented Dec 20, 2022 •
edited
Loading

The client seemingly has no way to know the real latest event id in the thread as the server doesn't give it a total order list of all the events, only the 1st-order related events (thread responses), edits and other 2nd-order relations are bundled inside the 1st-order relations' unsigned fields.

This issue impacts all users & devices who were not open/online at the time a thread response gets edited, all the while until another response is added to the thread. Based on this and how common edits are, I'd argue most users will see this from every now and then, so this sits just below O-Frequent given it isn't "regularly"

@germain-gg germain-gg self-assigned this Dec 21, 2022
@germain-gg
Copy link
Contributor

germain-gg commented Dec 21, 2022 •
edited
Loading

This has been regressed caused by matrix-org/matrix-js-sdk#2856

The canContain function in addEventToTimeline calls eventShoudLiveIn, and it does not figure out the threadId. It then incorrectly filters out the m.replace event...

Currently trying to find an appropriate fix for this issue

@t3chguy
Copy link
Member

t3chguy commented Dec 21, 2022

@gsouquet does that apply for threads entirely loaded from the server? I doubt the full m.replace event will be received in those cases

@germain-gg germain-gg mentioned this issue Dec 21, 2022
Merged
3 tasks
su-ex added a commit to SchildiChat/matrix-js-sdk that referenced this issue Jan 19, 2023
@su-ex
Merge tag 'v23.1.0' into sc
e6c127a 
* Remove extensible events v1 field population on legacy events ([\matrix-org#3040](matrix-org#3040)).
* Improve hasUserReadEvent and getUserReadUpTo realibility with threads ([\matrix-org#3031](matrix-org#3031)). Fixes element-hq/element-web#24164.
* Remove video track when muting video ([\matrix-org#3028](matrix-org#3028)). Fixes element-hq/element-call#209.
* Make poll start event type available (PSG-962) ([\matrix-org#3034](matrix-org#3034)).
* Add alt event type matching in Relations model ([\matrix-org#3018](matrix-org#3018)).
* Remove usage of v1 Identity Server API ([\matrix-org#3003](matrix-org#3003)).
* Add `device_id` to `/account/whoami` types ([\matrix-org#3005](matrix-org#3005)).
* Implement MSC3912: Relation-based redactions ([\matrix-org#2954](matrix-org#2954)).
* Introduce a mechanism for using the rust-crypto-sdk ([\matrix-org#2969](matrix-org#2969)).
* Support MSC3391: Account data deletion ([\matrix-org#2967](matrix-org#2967)).
* Fix threaded cache receipt when event holds multiple receipts ([\matrix-org#3026](matrix-org#3026)).
* Fix false key requests after verifying new device ([\matrix-org#3029](matrix-org#3029)). Fixes element-hq/element-web#24167 and element-hq/element-web#23333.
* Avoid triggering decryption errors when decrypting redacted events ([\matrix-org#3004](matrix-org#3004)). Fixes element-hq/element-web#24084.
* bugfix: upload OTKs in sliding sync mode ([\matrix-org#3008](matrix-org#3008)).
* Apply edits discovered from sync after thread is initialised ([\matrix-org#3002](matrix-org#3002)). Fixes element-hq/element-web#23921.
* Sliding sync: Fix issue where no unsubs are sent when switching rooms ([\matrix-org#2991](matrix-org#2991)).
* Threads are missing from the timeline ([\matrix-org#2996](matrix-org#2996)). Fixes element-hq/element-web#24036.
* Close all streams when a call ends ([\matrix-org#2992](matrix-org#2992)). Fixes element-hq/element-call#742.
* Resume to-device message queue after resumed sync ([\matrix-org#2920](matrix-org#2920)). Fixes matrix-org/element-web-rageshakes#17170.
* Fix browser entrypoint ([\matrix-org#3051](matrix-org#3051)). Fixes matrix-org#3013.
* Fix failure to start in firefox private browser ([\matrix-org#3058](matrix-org#3058)). Fixes element-hq/element-web#24216.
* Correctly handle limited sync responses by resetting the thread timeline ([\matrix-org#3056](matrix-org#3056)). Fixes element-hq/element-web#23952.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@germain-gg germain-gg

Labels
A-Message-Editing A-Notifications A-Threads O-Occasional Affects or can be seen by some users regularly or most users rarely S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Apply edits discovered from sync after thread is initialised matrix-org/matrix-js-sdk
4 participants
@germain-gg @t3chguy @robintown @gaelgatelement