search

element-hq / element-x-android

Android Matrix messenger application using the Matrix Rust Sdk and Jetpack Compose
GNU Affero General Public License v3.0
1.08k stars 155 forks source link

When displaying lots of notifications, latest ones won't appear #1052

Open jmartinesp opened 1 year ago

jmartinesp commented 1 year ago

Steps to reproduce

  1. Get the app on background using an account with lots of rooms.
  2. Get > 20/25/50 notifications displayed for the app (it depends on the vendor implementation). Please note these are different notifications, so i.e. having 20 messages in a single room would still count as a single notification, if I'm not mistaken.

Outcome

What did you expect?

It should either display all the notifications or the latest ones.

What happened instead?

Android OS has a max number of notifications that can be displayed at the same time per app to prevent spam, and once reached it won't display new notifications until one of the previous ones is dismissed. Given our implementation of the notification event queue, this can result in new notifications not being displayed while old ones are still present in the notification drawer.

There is also no way to know if posting a notification has failed, so we need to proactively limit the number of notifications we display. When posting a notification fails, we just get this message in the logs:

E NotificationService: Package has already posted or enqueued XX notifications.  Not showing more.

Your phone model

OnePlus 6T

Operating system version

Android 11

Application version and app store

Element X 0.1.2-nightly (also on develop)

Homeserver

matrix.org

Will you send logs?

No

Are you willing to provide a PR?

Yes

bmarty commented 1 year ago

I am seeing the GitHub issue right now, and I do not think there is an easy way to fix this. It's also worth noting that if the user dismiss the notification one by one (i.e. room by room), or using "clear all", the notifications in the queue will then be displayed, so nothing is lost. Not sure when the summary notification is dismiss though, I think in this case, we just empty the queue. But I guess the user is expecting the current behavior, even if they may miss a message.

What do you propose?

jmartinesp commented 1 year ago

If I recall correctly, in the code we had some 'cache' of notifications we use to store/read previous notifications to create the 'conversation' notifications. When we read them I think we're displaying the oldest ones first, but maybe we should be doing it the other way around, display always the latest ones with a limit of 20 notifications in the drawer?

I'm not sure how feasible this could be, I haven't looked at that code for a long time.

jmartinesp commented 4 months ago

I think after the changes in the notifications on Android this might no longer be an issue since we're not restoring the old notifications anymore. It wouldn't hurt to double check though.