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

Ping in a thread followed by a non-ping makes the room go red->unread #25523

Closed
turt2live opened this issue Jun 5, 2023 · 3 comments · Fixed by matrix-org/matrix-js-sdk#4106
Closed

Ping in a thread followed by a non-ping makes the room go red->unread #25523

turt2live opened this issue Jun 5, 2023 · 3 comments · Fixed by matrix-org/matrix-js-sdk#4106
Assignees
@dbkr
Labels
A-Notifications A-Threads O-Occasional Affects or can be seen by some users regularly or most users rarely S-Critical Prevents work, causes data loss and/or has no workaround T-Defect

Comments

@turt2live
Copy link
Member

Steps to reproduce

  1. Have an (encrypted) room set to Mentions Only
  2. Start a thread, sending a couple messages yourself
  3. Without looking at the room, have someone else reply to one of your messages in the thread, pinging you as a result.
  4. Again, without looking at the room, have that same person send a followup, non-ping, message

Outcome

What did you expect?

The room to stay red.

What happened instead?

It went from red at step 3 to unread (dot) at step 4.

Operating system

Windows 11

Application version

Element Nightly version: 0.0.1-nightly.2023060501 Olm version: 3.2.14

How did you install the app?

The Internet

Homeserver

t2l.io

Will you send logs?

Yes
@kittykat kittykat added S-Critical Prevents work, causes data loss and/or has no workaround A-Notifications O-Occasional Affects or can be seen by some users regularly or most users rarely A-Threads labels Jun 8, 2023
@t3chguy
Copy link
Member

t3chguy commented Aug 2, 2023

image
image
image
image

I can still reproduce this

@dbkr
Copy link
Member

dbkr commented Mar 11, 2024

I can't repro this now: the room stays red in the TAC as expected.

@dbkr dbkr closed this as completed Mar 11, 2024
@dbkr
Copy link
Member

dbkr commented Mar 11, 2024

wait, yes I can

@dbkr dbkr reopened this Mar 11, 2024
dbkr added a commit to matrix-org/matrix-js-sdk that referenced this issue Mar 11, 2024
@dbkr
Fix highlights from threads disappearing on new messages
6be10b7 
This changes interface of Room, so this is a BREAKING CHANGE.

Correctly mirrors the logic we use for room notifications for thread
notifications, ie. set only the total notifications count from the
server if it's zero.

I'm not delighted with this since it ends up with function on room
whose contract is to do something frankly, deeply weird and
unintuitive. However, this is the hack we use for room notifications
and it, empirically, works well enough. To do better, we'd need much
more complex logic to overlay notification counts for decrypted messages.

Fixes element-hq/element-web#25523
@dbkr dbkr mentioned this issue Mar 11, 2024
Merged
4 tasks
github-merge-queue bot pushed a commit to matrix-org/matrix-js-sdk that referenced this issue Mar 12, 2024
@dbkr
Fix highlights from threads disappearing on new messages (#4106)
d68ccc2 
* Fix highlights from threads disappearing on new messages

This changes interface of Room, so this is a BREAKING CHANGE.

Correctly mirrors the logic we use for room notifications for thread
notifications, ie. set only the total notifications count from the
server if it's zero.

I'm not delighted with this since it ends up with function on room
whose contract is to do something frankly, deeply weird and
unintuitive. However, this is the hack we use for room notifications
and it, empirically, works well enough. To do better, we'd need much
more complex logic to overlay notification counts for decrypted messages.

Fixes element-hq/element-web#25523

* Add tests for the special notification behaviour in syncing
github-merge-queue bot pushed a commit to matrix-org/matrix-js-sdk that referenced this issue Mar 13, 2024
@dbkr
Fix highlights from threads disappearing on new messages (#4106)
2f1ae5c 
* Fix highlights from threads disappearing on new messages

This changes interface of Room, so this is a BREAKING CHANGE.

Correctly mirrors the logic we use for room notifications for thread
notifications, ie. set only the total notifications count from the
server if it's zero.

I'm not delighted with this since it ends up with function on room
whose contract is to do something frankly, deeply weird and
unintuitive. However, this is the hack we use for room notifications
and it, empirically, works well enough. To do better, we'd need much
more complex logic to overlay notification counts for decrypted messages.

Fixes element-hq/element-web#25523

* Add tests for the special notification behaviour in syncing
github-merge-queue bot pushed a commit to matrix-org/matrix-js-sdk that referenced this issue Mar 20, 2024
@dbkr
Fix highlights from threads disappearing on new messages (#4106)
6c8ddca 
* Fix highlights from threads disappearing on new messages

This changes interface of Room, so this is a BREAKING CHANGE.

Correctly mirrors the logic we use for room notifications for thread
notifications, ie. set only the total notifications count from the
server if it's zero.

I'm not delighted with this since it ends up with function on room
whose contract is to do something frankly, deeply weird and
unintuitive. However, this is the hack we use for room notifications
and it, empirically, works well enough. To do better, we'd need much
more complex logic to overlay notification counts for decrypted messages.

Fixes element-hq/element-web#25523

* Add tests for the special notification behaviour in syncing
github-merge-queue bot pushed a commit to matrix-org/matrix-js-sdk that referenced this issue Mar 21, 2024
@dbkr
Fix highlights from threads disappearing on new messages (#4106)
3b32301 
* Fix highlights from threads disappearing on new messages

This changes interface of Room, so this is a BREAKING CHANGE.

Correctly mirrors the logic we use for room notifications for thread
notifications, ie. set only the total notifications count from the
server if it's zero.

I'm not delighted with this since it ends up with function on room
whose contract is to do something frankly, deeply weird and
unintuitive. However, this is the hack we use for room notifications
and it, empirically, works well enough. To do better, we'd need much
more complex logic to overlay notification counts for decrypted messages.

Fixes element-hq/element-web#25523

* Add tests for the special notification behaviour in syncing
github-merge-queue bot pushed a commit to matrix-org/matrix-js-sdk that referenced this issue Mar 21, 2024
@dbkr
Fix highlights from threads disappearing on new messages (#4106)
4ba1341 
* Fix highlights from threads disappearing on new messages

This changes interface of Room, so this is a BREAKING CHANGE.

Correctly mirrors the logic we use for room notifications for thread
notifications, ie. set only the total notifications count from the
server if it's zero.

I'm not delighted with this since it ends up with function on room
whose contract is to do something frankly, deeply weird and
unintuitive. However, this is the hack we use for room notifications
and it, empirically, works well enough. To do better, we'd need much
more complex logic to overlay notification counts for decrypted messages.

Fixes element-hq/element-web#25523

* Add tests for the special notification behaviour in syncing

* Correctly copy the room logic for reseting notifications

We were always ignoring the highlight count, even for encrypted rooms,
which was broken because we don't do the local calculation for unencrypted
rooms.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dbkr dbkr

Labels
A-Notifications A-Threads O-Occasional Affects or can be seen by some users regularly or most users rarely S-Critical Prevents work, causes data loss and/or has no workaround T-Defect
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix highlights from threads disappearing on new messages matrix-org/matrix-js-sdk
4 participants
@kittykat @dbkr @turt2live @t3chguy