Closed Bug 1486432 Opened Last year Closed Last year

Leanplum push notifications are not received on devices with Android O or above

Categories

(Firefox for Android :: General, defect)

Firefox 63
All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 63
Tracking Status
firefox61 --- unaffected
firefox62 --- unaffected
firefox63 + fixed

People

(Reporter: bsurd, Assigned: andrei.a.lazar)

References

(Blocks 1 open bug)

Details

(Whiteboard: --do_not_change--[priority:high])

Attachments

(1 file, 1 obsolete file)

Device(s):
 - Samsung Galaxy S8 (Android 8);
 - Google Pixel (Android 9);
 - Nexus 5 (Android 6.0.1).

Steps to reproduce:
 1. Install Nightly and turn on leanplum-start in about:experiments;
 2. From the leanplum dashboard send or trigger any push notification.

Expected result:
 A push notification is received on the device.

Actual result:
 A push notification is not received on the device.

Notes:
 Only android O and above seem to be affected by this, push notifications were received on the Nexus 5 running Android 6.
Assignee: nobody → vlad.baicu
Status: NEW → ASSIGNED
Whiteboard: --do_not_change--[priority:high]
Assignee: vlad.baicu → andrei.a.lazar
Ryan this will have to get uplifted.
Marcia and Ryan FYI.
Note that 63 still has another ~1w of time before going to Beta, so there's still time to fix this without needing an uplift. Tracking it to keep it on the radar at least.
Provided a valid default icon for push notifications in Leanplum required for fallback strategy.
Added default notification channel for Leanplum's push notifications.
No longer blocks: New_Tab_Deeplink
Bram, we might need some help for deciding on the proper name for this notification channel, too.
See https://phabricator.services.mozilla.com/D4426#inline-16027 and https://phabricator.services.mozilla.com/D4426#inline-16028.
Flags: needinfo?(bram)
Attachment #9004497 - Attachment is obsolete: true
Added a default channel for Leanplum push notifications as fallback to cover the cases where we receive
push notifications from server with an invalid channel and the case where there are no channels configured
on server.

Provided a valid default icon for push notifications in Leanplum required for fallback strategy.
Comment on attachment 9005273 [details]
Bug 1486432 Leanplum push notifications are not received on devices with Android O or above r=JanH

Francesco Lodolo [:flod] has approved the revision.
Attachment #9005273 - Flags: review+
Comment on attachment 9005273 [details]
Bug 1486432 Leanplum push notifications are not received on devices with Android O or above r=JanH

Jan Henning [:JanH] has approved the revision.
Attachment #9005273 - Flags: review+
Also NI'ing Brian Jones (copy) on this.

Is the notification channel name a user-visible string? If so, under what circumstances?
Flags: needinfo?(jh+bugzilla)
Flags: needinfo?(brjones)
Flags: needinfo?(andrei.a.lazar)
It is visible in Android's notification settings for our app on Android O and above.
Flags: needinfo?(jh+bugzilla)
(In reply to Jan Henning [:JanH] from comment #5)
> Bram, we might need some help for deciding on the proper name for this
> notification channel, too.
> See https://phabricator.services.mozilla.com/D4426#inline-16027 and
> https://phabricator.services.mozilla.com/D4426#inline-16028.

Hi Jan,

See bug 1479532 comment 20 for my advise.

If I read correctly, it sounds like we’ll have four different notification channels:

1. What’s new in Firefox (we already have this)
2. One-off notifications that’s high priority but don’t fit any category (new, proposed in bug 1479532)
3. Any other notifications not fit for more specific channels (new, proposed in bug 1479532)
4. Leanplum push notifications (new, proposed in this bug)

Is this correct?

I will defer to Brian and Michelle’s advice here, but having four channels seems like a bit too much to manage.


For something simpler, I recommend having a maximum of just 3 channels (even this may still be too much):

1. High priority, one-off notifications. New title: Warnings. Description: Warn you about Firefox errors.

2. What’s new in Firefox + any other notifications not fit for specific channels. I propose to combine these two into one “Medium level” notification channel. New title: ??? (depends on what those other notifications may be. Some sample strings would help.)
 
3. Leanplum push notifications. New title: Snippets. Description: Updates from Mozilla and Firefox.


For reference, here’s the In Product Communication Strategy that we should follow: https://docs.google.com/presentation/d/1H5A-obzXXIL0AX4BJRdcqFRc7IkE9ajOJVFCYBSB5Ts/edit
Flags: needinfo?(bram)
Flags: needinfo?(andrei.a.lazar)
Keywords: checkin-needed
Some review comments are still unaddressed.
Keywords: checkin-needed
Hardware: ARM → All
@Bram: See bug 1479532 comment 22 for my detailed answer.
Made requested review changes.
Keywords: checkin-needed
andrei: Hi, I couldn't land your patch. Please have a look at this:

Error: We're sorry, Autoland could not rebase your commits for you automatically. Please manually rebase your commits and try again. (255, 'applying /tmp/tmp_CEhyA\npatching file mobile/android/base/strings.xml.in\nHunk #1 succeeded at 653 with fuzz 1 (offset 0 lines).\npatching file mobile/android/base/java/org/mozilla/gecko/notifications/NotificationHelper.java\nHunk #1 FAILED at 104\n1 out of 4 hunks FAILED -- saving rejects to file mobile/android/base/java/org/mozilla/gecko/notifications/NotificationHelper.java.rej\nabort: patch failed to apply', '')
Flags: needinfo?(andrei.a.lazar)
Keywords: checkin-needed
Fixed merge conflicts.
Flags: needinfo?(andrei.a.lazar)
Keywords: checkin-needed
Pushed by ebalazs@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a36fdfd9fcf1
Leanplum push notifications are not received on devices with Android O or above r=JanH,flod
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/a36fdfd9fcf1
Status: ASSIGNED → RESOLVED
Closed: Last year
Resolution: --- → FIXED
Target Milestone: --- → Firefox 63
Blocks: 1488874
NI moved over to bug 1488874.
Flags: needinfo?(brjones)
You need to log in before you can comment on or make changes to this bug.