Closed Bug 1392224 Opened 3 years ago Closed 3 years ago

Make Fennec the default handler for Leanplum Push Notification

Categories

(Firefox for Android :: Metrics, enhancement)

enhancement
Not set
normal

Tracking

()

RESOLVED FIXED
Firefox 57
Tracking Status
firefox57 --- fixed

People

(Reporter: cnevinchen, Assigned: cnevinchen)

References

(Blocks 1 open bug)

Details

(Whiteboard: [FNC][SPT57.2][INT][LP_M2][SP=X, 3])

Attachments

(1 file)

We have two ways to let fennec be the default handler for Push Notification.
1. Through intent string in Leanplum backend
 => This don't need to change Leanplum SDK, but will need a lot of maintanace effort for Leaplum setup. And Leanplum SDK has bug in it since queryIntentActivities() may sometimes return null list. Thus make no activity can handle the action_vew intent.

2. Through the code we handle deep link
We can avoid above problems but need to manually upstream the SDK( and we already did).

The long term fix maybe submit a PR to Leanplum and see if we can use resolveActivity() instead. But for now I want to make it as simple as possible.
Whiteboard: [FNC][SPT57.2][INT][LP_M2]
Blocks: 1351571
Comment on attachment 8899422 [details]
Bug 1392224 - Make Fennec the default handler for Leanplum Push Notification.

https://reviewboard.mozilla.org/r/170696/#review176140
Attachment #8899422 - Flags: review?(max) → review+
Assignee: nobody → cnevinchen
Comment on attachment 8899422 [details]
Bug 1392224 - Make Fennec the default handler for Leanplum Push Notification.

https://reviewboard.mozilla.org/r/170696/#review176150

::: mobile/android/thirdparty/com/leanplum/LeanplumPushService.java:444
(Diff revision 1)
>      String action = notification.getString(Keys.PUSH_MESSAGE_ACTION);
>      if (action != null && action.contains(OPEN_URL)) {
>        Intent deepLinkIntent = getDeepLinkIntent(notification);
> -      if (deepLinkIntent != null && activityHasIntent(context, deepLinkIntent)) {
> +      if (deepLinkIntent != null) {
> +        // if it's a deep link, always use Fennec to start it
> +        deepLinkIntent.setPackage(context.getPackageName());

What's the difference between this line and inside activityHasIntent?
Attachment #8899422 - Flags: review+ → review?(max)
Comment on attachment 8899422 [details]
Bug 1392224 - Make Fennec the default handler for Leanplum Push Notification.

https://reviewboard.mozilla.org/r/170696/#review176578
Attachment #8899422 - Flags: review?(max) → review+
Pushed by nechen@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7fa4d82e8c09
Make Fennec the default handler for Leanplum Push Notification. r=maliu
https://hg.mozilla.org/mozilla-central/rev/7fa4d82e8c09
Status: NEW → RESOLVED
Closed: 3 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 57
Whiteboard: [FNC][SPT57.2][INT][LP_M2] → [FNC][SPT57.2][INT][LP_M2][SP=X, 3]
You need to log in before you can comment on or make changes to this bug.