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.

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

Use explicit PendingIntents for notification actions #1190

Merged

Conversation

roberi
Copy link
Contributor

@roberi roberi commented Jul 5, 2024

Fixes #1189

Starting with Android 14, implicit intents are restricted for internal app components. This commit explicitly sets the package name for PendingIntents used as notification actions to ensure compatibility with the behavior changes outlined in: https://developer.android.com/about/versions/14/behavior-changes-14#security

Starting with Android 14, implicit intents are restricted for internal app components. This commit explicitly sets the package name for PendingIntents used as notification actions to ensure compatibility with the behavior changes outlined in:
https://developer.android.com/about/versions/14/behavior-changes-14#security
@gerhardol
Copy link
Collaborator

Approved, working fine

There are two occurrences each in app/src/main/org/runnerup/tracker/component/TrackerPebble.java:85 and app/src/main/org/runnerup/tracker/component/TrackerPebble.java:479
Even if Wear is dropped right now and there are likely a minimum of Pebble users, I suggest that is changed.
I would have expected that app/src/main/org/runnerup/tracker/component/TrackerPebble.java:735 :759 would be affected too

I thought I had checked this already...

@roberi
Copy link
Contributor Author

roberi commented Jul 6, 2024

There are two occurrences each in app/src/main/org/runnerup/tracker/component/TrackerPebble.java:85...

I agree, this we should change.

But I'm unsure why LocalBroadcastManager is used to broadcast RESUME_WORKOUT and PAUSE_WORKOUT and context.sendBroadcast() is used to broadcast START_WORKOUT.

I think we could either just change startBroadcastIntent.setPackage(null) to startBroadcastIntent.setPackage(context.getPackageName()) or to replace

Intent startBroadcastIntent = new Intent()
    .setAction(org.runnerup.common.util.Constants.Intents.START_WORKOUT);
startBroadcastIntent.setPackage(context.getPackageName());
context.sendBroadcast(startBroadcastIntent);

with sendLocalBroadcast(org.runnerup.common.util.Constants.Intents.START_WORKOUT)

Any thoughts about this?

@roberi
Copy link
Contributor Author

roberi commented Jul 6, 2024

... and app/src/main/org/runnerup/tracker/component/TrackerPebble.java:479

I assume you mean app/src/wear/org/runnerup/tracker/component/TrackerWear.java:479?

I agree, but same question here. Do we use contextsendBroadcast() or LocalBroadcastManager?

@roberi
Copy link
Contributor Author

roberi commented Jul 6, 2024

I would have expected that app/src/main/org/runnerup/tracker/component/TrackerPebble.java:735 :759 would be affected too

You must mean some other class than TrackerPebble (since it only has 179 lines)?

@gerhardol
Copy link
Collaborator

Failed copy...
app/src/main/org/runnerup/view/StartActivity.java:735 :759
app/src/wear/org/runnerup/tracker/component/TrackerWear.java:479

I think we could either just change startBroadcastIntent.setPackage(null) to startBroadcastIntent.setPackage(context.getPackageName()) or to replace
with sendLocalBroadcast(org.runnerup.common.util.Constants.Intents.START_WORKOUT)

Any thoughts about this?

No opinion right now. Should try to connect to Pebble users.
Maybe just open an issue about it for now and merge this as is.

@gerhardol gerhardol merged commit cf3b059 into jonasoreland:master Jul 9, 2024
@roberi roberi deleted the fix/i1189-notification-actions branch July 10, 2024 15:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Notification action buttons not working on Android 14
2 participants