Fix notifications before the targetSdk bump (channel, PendingIntent mutability, POST_NOTIFICATIONS)#10
Open
xroche wants to merge 2 commits into
Open
Fix notifications before the targetSdk bump (channel, PendingIntent mutability, POST_NOTIFICATIONS)#10xroche wants to merge 2 commits into
xroche wants to merge 2 commits into
Conversation
Three separate breakages in sendSystemNotification(), the single sink for every notification the app posts. All three are inert at targetSdk 25, so this lands now and the target bump stays a one-variable change. The PendingIntent is the severe one: a bare 0 flag throws from API 31 on, and one of the three callers is RunnerFragment.onDestroy, so the app would crash on every aborted crawl rather than merely lose a notification. The extras are read back only by us, so FLAG_IMMUTABLE is the right one. Lint has been reporting this as UnspecifiedImmutableFlag all along; it went unnoticed because abortOnError is false. The channel is mandatory from API 26 on, and without it the system drops the notification with nothing but a log line. NotificationChannelCompat registers it with no version gate. POST_NOTIFICATIONS is enforced by the OS from Android 13, but targeting 33+ also removes the automatic prompt, so an explicit request is the only way to get anything at all. It is asked at crawl start rather than at launch because two refusals make it permanent, and a prompt at launch has nothing to point at yet. setContentInfo() went with the rewrite: it has been ignored since API 24. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
…t time Review of the first commit found the request could fire twice. The activity has no configChanges, so a rotation mid-crawl restores pane_id, re-enters the progress pane and re-asks; a user who declined once and reflexively declines the repeat has denied POST_NOTIFICATIONS for good. A field would die with the instance like pane_id does, so the latch lives in the existing preferences file. Creating the channel in onCreate was also wrong in a subtler way. androidx branches on the device's SDK_INT, never on targetSdk, so the channel really was created on every launch of every API 26+ device -- and for an app targeting 32 or lower, creating a channel is precisely what makes the system show its own permission dialog. Startup is the worst moment to ask, being the one point where there is no mirror to announce. Registering it in sendSystemNotification() instead keeps it ahead of every post, including the save-failure one that can fire without any crawl. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Signed-off-by: Xavier Roche <roche@httrack.com>
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
1 participant
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.
Three things that break notifications at targetSdk 36, landing ahead of the bump so that the bump itself stays a one-variable change.
The
PendingIntentis the one that matters: a bare0flag throws from API 31 on, and one of the three callers isRunnerFragment.onDestroy, so this is a crash on every aborted crawl rather than a lost notification. Lint has been flagging it asUnspecifiedImmutableFlagall along, unnoticed becauseabortOnErroris false. The other two are silent by nature: a channel-less notification is dropped from API 26 with only a log line, andPOST_NOTIFICATIONSneeds an explicit request, since targeting 33+ removes the automatic prompt.Everything lands in
sendSystemNotification(), the single sink for every notification the app posts. (showNotification()is a Toast despite the name, so its call sites are untouched.) The permission is requested at crawl start rather than at launch, once per install and latched in preferences: the activity has noconfigChanges, so a rotation would otherwise re-enter the progress pane and re-ask, and the second refusal is the one that sticks for good. The channel is registered at post time for the same reason, since creating a channel is what makes the system prompt an app targeting 32 or lower, and because the save-failure notification can fire without any crawl having run.This is not inert on current devices, contrary to what the first commit assumed: androidx branches on the device
SDK_INTand never on targetSdk, so on any API 26+ device the notifications now carry a real channel and the app gains an entry in the system notification settings. Master is in fact silently broken on Android 13 today, creating no channel and therefore never getting the system's own permission dialog, so a fresh install posts nothing at all. This PR is what fixes that.Verified by differential: lint reports
UnspecifiedImmutableFlagonce on master and zero here, andPOST_NOTIFICATIONSappears in the merged manifest of the built APK withtargetSdkVersionstill 25. The channel and the prompt cannot be exercised without a device, so they rest on the API contract rather than on observation.Left alone deliberately, each wanting its own PR: the abort
PendingIntentreuses request code0withoutFLAG_UPDATE_CURRENT, so a second abort notification carries the first one's extras and tapping it restores the wrong project (pre-existing, master behaves identically), and the mirror-finished intent carries no extras at all, which is what itsFIXMEsays.