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

Status: RESOLVED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Bogdan Surd, QA [:bsurd, NI]]

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.

Comment 1

[:sdaswani]

Ryan this will have to get uplifted.

Comment 2

[:sdaswani]

Marcia and Ryan FYI.

Comment 3

[Ryan VanderMeulen [:RyanVM]]

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.

Comment 4

[Andrei Lazar]

Attachment: Bug 1486432 Leanplum push notifications are not received on devices with Android O or above r=sdaswani

Provided a valid default icon for push notifications in Leanplum required for fallback strategy.
Added default notification channel for Leanplum's push notifications.

Comment 5

[Jan Henning [:JanH]]

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.

Comment 6

[Andrei Lazar]

Attachment: Bug 1486432 Leanplum push notifications are not received on devices with Android O or above r=JanH

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 7

[Francesco Lodolo [:flod]]

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.

Comment 8

[Jan Henning [:JanH]]

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.

Comment 9

[:sdaswani]

Also NI'ing Brian Jones (copy) on this.

Is the notification channel name a user-visible string? If so, under what circumstances?

Comment 10

[Jan Henning [:JanH]]

It is visible in Android's notification settings for our app on Android O and above.

Comment 11

[Jan Henning [:JanH]]

See also https://developer.android.com/training/notify-user/channels.

Comment 12

[Bram Pitoyo [:bram]]

(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

Comment 13

[Jan Henning [:JanH]]

Some review comments are still unaddressed.

Comment 14

[Jan Henning [:JanH]]

@Bram: See bug 1479532 comment 22 for my detailed answer.

Comment 15

[Andrei Lazar]

Made requested review changes.

Comment 16

[Eliza Balazs [:ebalazs_]]

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', '')

Comment 17

[Andrei Lazar]

Fixed merge conflicts.

Comment 18

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/a36fdfd9fcf1
Leanplum push notifications are not received on devices with Android O or above r=JanH,flod

Comment 19

[Andrei Ciure[:aciure]]

https://hg.mozilla.org/mozilla-central/rev/a36fdfd9fcf1

Comment 20

[Jan Henning [:JanH]]

NI moved over to bug 1488874.