fix(native): stop reporting a granted notification permission for a muted Android app - #655
Open
pal-tamas wants to merge 1 commit into
Open
fix(native): stop reporting a granted notification permission for a muted Android app#655pal-tamas wants to merge 1 commit into
pal-tamas wants to merge 1 commit into
Conversation
…uted Android app
INotifications asked only whether the app HELD POST_NOTIFICATIONS, and below API 33 --
where no such permission exists -- short-circuited to Granted outright. But the per-app
notification toggle in Settings is independent of the permission, has existed since API
19, and turning it off makes NotificationManager.Notify a SILENT no-op.
So a user who muted the app got Granted from PermissionAsync(), ShowAsync returned
without throwing, and nothing ever appeared. Nothing in the API said otherwise.
AreNotificationsEnabled is the one call that sees that toggle, and it exists from API 24
-- the android head's own SupportedOSPlatformVersion -- so it covers the whole supported
range without a version guard. It is now checked first.
A muted app reports Denied rather than Default: the way back is the Settings screen, not
a prompt, which is exactly what Denied means in the web contract this backend mirrors
("blocked until the user changes the setting"). RequestPermissionAsync returns it too,
instead of claiming a grant no prompt could produce -- it was answering Granted
unconditionally below API 33, before it ever consulted the current state.
Behaviour change: ShowAsync on a muted app now throws InvalidOperationException, like
any other ungranted permission, where it previously returned and quietly showed nothing.
That is the documented contract of this backend ("a denied permission throws"), and the
silent version is what made the bug invisible.
The equivalent gap in the IPermissions native backend was closed in #644; the comment
there points here, so the two now agree about the same app.
Android compiles only in CI's native jobs (no android workload locally), so those are
the gate for this one.
Closes #649.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
INotificationsasked only whether the app heldPOST_NOTIFICATIONS, and below API 33 — where nosuch permission exists — short-circuited to
Grantedoutright:But the per-app notification toggle in Settings is independent of the permission, exists on every
supported version, and turning it off makes
NotificationManager.Notifya silent no-op. So a user whomuted the app got
GrantedfromPermissionAsync(),ShowAsyncreturned without throwing, and nothingever appeared — with nothing in the API saying otherwise.
AreNotificationsEnabledis the one call that sees that toggle. It exists from API 24 — the androidhead's own
SupportedOSPlatformVersion— so it covers the whole supported range with no version guard, andit's reached through the existing
AndroidNotifications.Managerhelper.Decisions worth flagging
Denied, notDefault. The way back is the Settings screen, not a prompt —which is exactly what
Deniedmeans in the web contract this backend mirrors ("blocked until the userchanges the setting").
Defaultwould tell a caller a request is still worth making.RequestPermissionAsyncreturns it too, rather than claiming a grant no prompt could produce. It wasanswering
Grantedunconditionally below API 33, before it ever consulted the current state; the mutedcheck now runs first.
ShowAsyncon a muted app now throwsInvalidOperationException, like any otherungranted permission, where it previously returned and quietly showed nothing. That is this backend's
documented contract ("a denied permission throws"), and the silent version is what made the bug
invisible in the first place.
Testing
dotnet formatclean,CI=true dotnet build Rask.slnx -warnaserror -m:1clean, full unit suite green,browser E2E 57/57 — run twice, since a single clean run can hide the two-build-mode
obj/hazardfix(playground): wait for the editor, and stop the E2E gate breaking its own publish #652 just fixed.
androidworkload locally), so those are the gatehere — as for every Android-side change. There is no unit-test seam:
Platforms/Android/**is excludedfrom the
net10.0TFM thatRask.Native.Testsbuilds against.the same basis on which the bug was filed.
Related
The identical gap in the
IPermissionsnative backend was closed in #644, whoseNativePermissionQuery.NotificationStatecomment points at this file — so once both land, the two APIsagree about the same app instead of contradicting each other.
Closes #649.