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

Fix notification open not launching main activity by default on app cold start #1276

Merged
merged 3 commits into from
Feb 5, 2021
Merged

Fix notification open not launching main activity by default on app cold start #1276

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0326d93
Fixed app open on cold start from notification
jkasten2 Feb 3, 2021
b27ac14
rm isTaskRoot & NEW_TASK flag from notifi open
jkasten2 Feb 3, 2021
14c340f
Clean up of startOrResumeApp
jkasten2 Feb 3, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 10 additions & 14 deletions OneSignalSDK/onesignal/src/main/java/com/onesignal/OneSignal.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2169,20 +2169,16 @@ static void applicationOpenedByNotification(@Nullable final String notificationI
sessionManager.onDirectInfluenceFromNotificationOpen(appEntryState, notificationId);
}

static boolean startOrResumeApp(Activity inContext) {
Intent launchIntent = inContext.getPackageManager().getLaunchIntentForPackage(inContext.getPackageName());
logger.debug("startOrResumeApp from context: " + inContext + " isRoot: " + inContext.isTaskRoot() + " with launchIntent: " + launchIntent);
// Make sure we have a launcher intent.
if (launchIntent != null) {
if (inContext.isTaskRoot()) {
inContext.startActivity(launchIntent);
} else {
launchIntent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
PendingIntent.getActivity(inContext, 0, launchIntent, 0);
}
return true;
}
return false;
static boolean startOrResumeApp(Activity activity) {
Intent launchIntent = activity.getPackageManager().getLaunchIntentForPackage(activity.getPackageName());
logger.debug("startOrResumeApp from context: " + activity + " isRoot: " + activity.isTaskRoot() + " with launchIntent: " + launchIntent);

// Not all apps have a launcher intent, such as one that only provides a homescreen widget
if (launchIntent == null)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How does this prevent the application from being launched from the launching activity when the application is in the background, instead of resuming the activity on top of the stack?
Doesn't this remove the behavior of emulating app icon click when the app is in the background when clicking a notification?

Copy link
Member Author

@jkasten2 jkasten2 Feb 3, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bug was due to missing a startActivity in the else of the isTaskRoot check. This was fixed in commit 0326d93 of this PR.

I noticed getLaunchIntentForPackage defaults to FLAG_ACTIVITY_NEW_TASK so the whole isTaskRoot check and setting of it isn't needed. Notes to this and ASOP source linked of this in commit b27ac14

I didn't notice any other behavior differences before and after this PR. See the manual test scenarios above for what I tested.

This PR is scoped to fixing this one specific cold start bug. However I did notice an additional resume issue which which I fixed in PR #1277.

return false;

activity.startActivity(launchIntent);
return true;
}

/**
Expand Down