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

Use the new POST_NOTIFICATIONS runtime permission on Android #5753

Closed
gnprice opened this issue Sep 1, 2023 · 0 comments · Fixed by #5761
Closed

Use the new POST_NOTIFICATIONS runtime permission on Android #5753

gnprice opened this issue Sep 1, 2023 · 0 comments · Fixed by #5761
Labels
a-Android a-notifications P1 high-priority

Comments

@gnprice
Copy link
Member

gnprice commented Sep 1, 2023

I think this is coupled with #5453, targeting Android 13 — that we'll have to make both changes at the same time. But giving it its own thread for ease of clearer discussion.

This new permission is documented here:
https://developer.android.com/develop/ui/views/notifications/notification-permission

What isn't documented is that if you target Android 13 (set targetSdkVersion to 33) without adding that permission in the manifest, then all notifications are blocked. We learned that the hard way:

Some discussion from @chrisbobbe in a chat thread:

It's a "runtime permission", which means we have to put up a permission prompt to the user. They ask that we do it in context, which sounds a lot like what Apple wants. To comply on iOS, we've been postponing that prompt until the first time /register completes.

I'm not sure it's a hard requirement that we do the permissions request in context. (Maybe we could just do it at startup?)

Also, one reason the iOS logic gets a little complicated (or at least different from Android; #5329) is that iOS withholds the device token until the dialog has been shown and accepted, but possibly Android doesn't withhold it like that.
@gnprice gnprice mentioned this issue Sep 1, 2023
Closed
chrisbobbe added a commit to chrisbobbe/zulip-mobile that referenced this issue Sep 11, 2023
@chrisbobbe
android: targetSdkVersion to 33, this time handling new notifs permis…
a44f580 
…sion

We bumped this to 33 in 6f44474 / v27.210, but then reverted it in
2e196e4 because of zulip#5753, "Use the new POST_NOTIFICATIONS runtime
permission on Android".

Bump it again, this time with a fix for that issue: we declare the
new permission and add a runtime function call to request it in
context.

For users, this means:

- If on iOS: No change.

- If below Android 13: No change. A permission request follows the
  creation of a notification channel, so effectively it happens on
  app startup:
    https://developer.android.com/develop/ui/views/notifications/notification-permission#new-apps

- For a new app install on Android 13, the permission will now be
  requested in context instead of at startup:
    https://developer.android.com/develop/ui/views/notifications/notification-permission#new-apps

- For existing app installations when Android is upgraded to 13 or
  later, the permission should automatically be granted without a
  prompt as long as notifications hadn't been explicitly disabled
  before the upgrade. This should still work across a "backup and
  restore":
    https://developer.android.com/develop/ui/views/notifications/notification-permission#existing-apps
  (However: we are tracking zulip#5484 as an unrelated cause of
  notifications breaking across a "backup and restore".)

If RN and Android decide it's helpful, a "rationale" is shown to the
user just before the new in-context permission request. That's a
modal showing a message we provide (see added code), with an OK
button to close the modal and proceed to Android's plain Yes/No
dialog for the permission request.

Fixes: zulip#5753
@chrisbobbe chrisbobbe mentioned this issue Sep 11, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
a-Android a-notifications P1 high-priority
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

android: targetSdkVersion to 33, this time handling new notifs permission chrisbobbe/zulip-mobile
1 participant
@gnprice