Skip to content
This repository has been archived by the owner on Apr 12, 2022. It is now read-only.

sticky notifications come back #2088

Closed
532910 opened this issue Mar 17, 2018 · 18 comments
Closed

sticky notifications come back #2088

532910 opened this issue Mar 17, 2018 · 18 comments

Comments

@532910
Copy link
Contributor

532910 commented Mar 17, 2018

after upgrade to 0.8.3 it was impossible to clear the notification about new message
but some time later it disappeares

@ylecollen
Copy link
Contributor

ylecollen commented Mar 19, 2018

@c-e-p-x-u-o

I assume i could explain your notification behaviour (because i'm the one who implemented it)

The notification is unswippable because it is linked to a service which is putin foreground.

When a push is received (when the application is in background), a sync request is triggered to retrieve the notified messages.

This service is put in foreground during this catchup (it might require until 30s to be done) to keep it alive ie avoid being killed asap by android (android >= 6 devices).

At the end of this catchup, the notification might disappear. (it seems it was your case).

So, you have many ways to fix it
1- you give the permission to the application to run in background : the notification won't be stuck anymore.
I added the permission request but NOBODY accepted it.
It could have been optional on android < 8 devices, but there were many failure cases (ie the sync fails and does not trigger the dedicated notifications)

The most boring ones were the missing incoming calls (and, please, don't tell us that you don't use the call feature, many users use it)

On android >= 8 devices, it is required. The background sync request cannot work without it.
Telegram added a permission request with extra information to explain why it is required.

2- you disable the background sync: the notifications will be something like "XXX unread messages"

i'm sure nobody would accept to do not receive notifications because of android restrictions

the fix could be

1- update
https://github.com/vector-im/riot-android/blob/master/vector/src/main/java/im/vector/util/PreferencesManager.java#L214 (replace M by O), and i'm sure you will have issue like 'i did not receive my notifications' or so (because the requests cannot be sent because the network connection is cut)

2- you ask the permission to "run in background" once (with description similar to telegram)
If the user does not grant it, you disable the background sync (and incoming call events will be lost)

3- you add a 2nd notofification like we used to have on fdroid, and many users will complain about having two Riot notifications

@532910
Copy link
Contributor Author

532910 commented Mar 19, 2018

Thank you for explanation, looks like my understanding of the process is correct.

At the end of this catchup, the notification might disappear. (it seems it was your case).
No it was not my case. I was not able to clear notification during 30 minutes after startup and was needed to kill riot.

My riot have ALL permission including battery (not optimized)
(Could you also explain the "phone" permission as it's most unclear)

@532910
Copy link
Contributor Author

532910 commented Mar 19, 2018

Moreover the stucked notification is not "Synchronizing" but the latest message.

@532910
Copy link
Contributor Author

532910 commented Mar 19, 2018

Sorry for confusing, I can't clear it for already three hours.

@532910
Copy link
Contributor Author

532910 commented Mar 20, 2018

I can't clear it even if I close riot. I need to kill riot by long back.

@ylecollen
Copy link
Contributor

@manuroe @Tyuoli
https://github.com/vector-im/riot-android/blob/master/vector/src/main/java/im/vector/services/EventStreamService.java#L274

i suspect you would need to update
(mNotificationState == NotificationState.INITIAL_SYNCING)
to
(mNotificationState == NotificationState.INITIAL_SYNCING) ll VectorApp.isAppInBackground()

@ylecollen
Copy link
Contributor

ylecollen commented Mar 20, 2018

@c-e-p-x-u-o
please send a bug report, manuroe or Tyuoli would be able to track the service state update.

"phone" permission -> i should have named it "android permissions"
It is to make the differences with Riot settings

@532910
Copy link
Contributor Author

532910 commented Mar 20, 2018

You mean send bug report from the app, right?
I found a way to reproduce it: just close riot during audio call.
And after the second voice call with the stucked notification from the first it's cleared automagically.

@532910
Copy link
Contributor Author

532910 commented Mar 20, 2018

r1r2

@ghost
Copy link

ghost commented Mar 20, 2018

I think that one is so Riot can detect when you're on a normal phone call. If Riot would ring for an incoming Matrix call while you're already on a normal call, that would be very annoying indeed.

@manuroe
Copy link
Member

manuroe commented Mar 20, 2018

@ylecollen, hello :)

When a push is received (when the application is in background), a sync request is triggered to retrieve the notified messages.
This service is put in foreground during this catchup (it might require until 30s to be done) to keep it alive ie avoid being killed asap by android (android >= 6 devices).
At the end of this catchup, the notification might disappear. (it seems it was your case).

I have now a doubt on this. This foreground notification, "Synchronising", seems to be displayed only for an initial sync (on login or on deleting the cache) by the code at:
https://github.com/vector-im/riot-android/blob/bfe8b9ca6178a87d5d9686716b71917930a3d470/vector/src/main/java/im/vector/services/EventStreamService.java#L884-L887

All this piece of code talks about initial sync and debugging it shows that we never pass in it when doing a background sync to fetch event message data.
No foreground service notification is actually created. The OS can kill the background sync task at anytime.

@manuroe
Copy link
Member

manuroe commented Mar 22, 2018

Wow. We have now a long list of related rageshakes but most of them are before 0.8.3 where notifications have been improved.

@532910
Copy link
Contributor Author

532910 commented Mar 22, 2018

I've send two bug reports from 0.8.3.

@Rejinderi
Copy link

The last message that was sent is stuck at my notification bar. It won't go away unless i restart my phone.

@532910
Copy link
Contributor Author

532910 commented Apr 3, 2018

@Rejinderi 0.8.5 version?

@Rejinderi
Copy link

@c-e-p-x-u-o precisely

@532910
Copy link
Contributor Author

532910 commented Apr 3, 2018

Confirm! I've send bugrepot from the riot app.

@manuroe
Copy link
Member

manuroe commented Apr 19, 2018

Related to #2130 that gives information on how we fix it.

@manuroe manuroe closed this as completed Apr 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants