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

Notification does not vanish if the message is read on another session #3347

Closed
bmarty opened this issue May 14, 2021 · 3 comments · Fixed by #4129
Closed

Notification does not vanish if the message is read on another session #3347

bmarty opened this issue May 14, 2021 · 3 comments · Fixed by #4129
Assignees
Labels
A-Notifications T-Defect Something isn't working: bugs, crashes, hangs and other reported problems

Comments

@bmarty
Copy link
Member

bmarty commented May 14, 2021

  • Bob has 2 sessions: one on Element Android, one on Element Web, and share a room with Alice.
  • Bob is viewing the room list on Element Android
  • Alice sends a message to Bob in room A
  • The notification appears on Bob's Android device.
  • Bob read the room in his Element Web session
  • The notification is still visible to Bob's Android device -> KO

The notification should be cancel on Bob's Android device.

It used to work, so this is regression.

During the while process Element Android is in foreground (so is syncing permanently). If Element Android app goes to background, I'm not sure we will receive a Push to update the read marker of Bob, but it is acceptable in this case.

@bmarty bmarty added T-Defect Something isn't working: bugs, crashes, hangs and other reported problems A-Notifications labels May 14, 2021
@bmarty bmarty self-assigned this May 14, 2021
@bmarty
Copy link
Member Author

bmarty commented Aug 31, 2021

I've noticed that the notification of one room is not dismissed when we open the room on the same device, which is probably a related issue.

@bmarty bmarty added this to the Sprint - Element 1.1.15 milestone Aug 31, 2021
@ouchadam ouchadam self-assigned this Sep 22, 2021
@ouchadam
Copy link
Contributor

I've been unable to reproduce, using a Google Play variant with the latest from develop cfd37bb and testing on a Pocofone f1 (API 29) and Samsung Fold (API 30) the notification correctly dismisses when focusing on the web client.

The notification also dismisses whilst in the app is in the background and the web client gains focus.

@ouchadam
Copy link
Contributor

ouchadam commented Sep 30, 2021

I think I've found the cause, other than having an invalid session or removing the room this is the only way I've been able to reproduce 🤞

the current logic relies on all the notification events existing within the same last chunk which means if a user gets enough messages in their timeline to start a new chunk it's now impossible for the app to match against the older events (the notification is cleared when all events for a roomId have been filtered by isMessageOutdated which solely relies on the last chunk)

tl;dr high traffic rooms cause us to lose track of the event id to remove

this might be the source of the regression

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Notifications T-Defect Something isn't working: bugs, crashes, hangs and other reported problems
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants