Closed Bug 1195692 Opened 5 years ago Closed 5 years ago

Android API 23 (M) removes deprecated Notification.setLatestEventInfo()

Categories

(Firefox for Android :: General, defect)

All
Android
defect
Not set

Tracking

()

RESOLVED FIXED
Firefox 44
Tracking Status
firefox43 --- affected
firefox44 --- fixed

People

(Reporter: sebastian, Assigned: sebastian)

References

(Blocks 1 open bug)

Details

Attachments

(3 files)

Notification.setLatestEventInfo() is deprecated and has been removed with Android API level 23:

> 1:52.55 /Users/sebastian/Projects/Mozilla/fx-team/mobile/android/base/updater/UpdateService.java:311: error: cannot find symbol
> 1:52.55             notification.setLatestEventInfo(this, getResources().getString(R.string.updater_start_title),
> 1:52.55                         ^
> 1:52.55   symbol:   method setLatestEventInfo(UpdateService,String,String,PendingIntent)
> 1:52.55   location: variable notification of type Notification
> 1:52.56 /Users/sebastian/Projects/Mozilla/fx-team/mobile/android/base/updater/UpdateService.java:344: error: cannot find symbol
> 1:52.56             notification.setLatestEventInfo(this, getResources().getString(R.string.updater_apply_title),
> 1:52.56                         ^
> 1:52.56   symbol:   method setLatestEventInfo(UpdateService,String,String,PendingIntent)
> 1:52.56   location: variable notification of type Notification
> 1:52.56 /Users/sebastian/Projects/Mozilla/fx-team/mobile/android/base/updater/UpdateService.java:485: error: cannot find symbol
> 1:52.57         notification.setLatestEventInfo(this, getResources().getString(R.string.updater_downloading_title_failed),
> 1:52.57                     ^
> 1:52.57   symbol:   method setLatestEventInfo(UpdateService,String,String,PendingIntent)
> 1:52.57   location: variable notification of type Notification
> 1:52.84 Note: com.google.android.gms.ads.internal.purchase.zzb accesses a declared method 'asInterface(android.os.IBinder)' dynamically
> 1:52.92 Note: com.google.android.gms.common.internal.DowngradeableSafeParcel accesses a field 'NULL' dynamically
> 1:52.92       Maybe this is program field 'com.google.android.gms.common.internal.safeparcel.SafeParcelable { java.lang.String NULL; }'
> 1:52.93       Maybe this is library field 'android.util.JsonToken { android.util.JsonToken NULL; }'
> 1:52.93       Maybe this is library field 'java.sql.Types { int NULL; }'
> 1:52.93       Maybe this is library field 'org.json.JSONObject { java.lang.Object NULL; }'
> 1:53.13 /Users/sebastian/Projects/Mozilla/fx-team/mobile/android/base/sync/CommandProcessor.java:284: error: cannot find symbol
> 1:53.13     notification.setLatestEventInfo(context, notificationTitle, uri, contentIntent);
> 1:53.13                 ^
> 1:53.13   symbol:   method setLatestEventInfo(Context,String,String,PendingIntent)
> 1:53.13   location: variable notification of type Notification

The deprecation annotation just says: "Use {@link Builder} instead."
UpdateService uses some more deprecated methods (like the Notification constructor). We probably should just migrate this class to use the notification builder. And maybe not even the one of the Android SDK but the one in the support library.
Assignee: nobody → s.kaspari
Status: NEW → ASSIGNED
Eugen: I picked you as reviewer because you've been the only one editing this file this year. Let me know if you want someone else to review the patch. :)
Attachment #8649213 - Flags: review?(esawin)
Now only NotificationHandler / AlertNotification is left. This might need a bit more work.
Attachment #8649225 - Flags: review?(michael.l.comella)
Comment on attachment 8649213 [details] [diff] [review]
1195692-UpdateService.patch

Review of attachment 8649213 [details] [diff] [review]:
-----------------------------------------------------------------

This looks good! From what I read, this has fallbacks, so we should be OK with older supported platforms, too.

::: mobile/android/base/updater/UpdateService.java
@@ +304,1 @@
>  

I think we can remove the blank line.
Attachment #8649213 - Flags: review?(esawin) → review+
Comment on attachment 8649225 [details] [diff] [review]
1195692-CommandProcessor.patch

Review of attachment 8649225 [details] [diff] [review]:
-----------------------------------------------------------------

::: mobile/android/base/sync/CommandProcessor.java
@@ +274,5 @@
>      notificationIntent.putExtra(TabQueueDispatcher.SKIP_TAB_QUEUE_FLAG, true);
>      PendingIntent contentIntent = PendingIntent.getActivity(context, 0, notificationIntent, 0);
> +
> +    NotificationCompat.Builder builder = new NotificationCompat.Builder(context);
> +    builder.setSmallIcon(R.drawable.flat_icon);

Should we add a bigIcon here (in a follow up)?
Attachment #8649225 - Flags: review?(michael.l.comella) → review+
(In reply to Michael Comella (:mcomella) from comment #5)
> Should we add a bigIcon here (in a follow up)?

So far I only tried to recreate the previous behavior. Now with the NotificationCompat class we actually could start to use some of the newer features where supported and make our notifications more appealing.
url:        https://hg.mozilla.org/integration/fx-team/rev/7bc736ff4529fd1c6f1abc31825ce56c2bc03ac0
changeset:  7bc736ff4529fd1c6f1abc31825ce56c2bc03ac0
user:       Sebastian Kaspari <s.kaspari@gmail.com>
date:       Fri Aug 21 11:52:25 2015 +0200
description:
Bug 1195692 - UpdateService: Use NotificationCompat.Builder to create notifications. r=esawin
url:        https://hg.mozilla.org/integration/fx-team/rev/62a945fe01762d7b7cc424e204ad5270f55218e1
changeset:  62a945fe01762d7b7cc424e204ad5270f55218e1
user:       Sebastian Kaspari <s.kaspari@gmail.com>
date:       Fri Aug 21 11:54:47 2015 +0200
description:
Bug 1195692 - CommandProcessor: Use NotificationCompat.Builder to create notifications. r=mcomella
The last call to setLatestEventInfo() is in NotificationHandler on AlertNotification objects. While trying to understand what this class is used for I found bug 893289 (with a WIP patch) about killing AlertNotification all together.
See Also: → 893289
Bug 1195692 - Replace AlertNotification with NotificationCompat.Builder based implementation. r?mcomella
Attachment #8665023 - Flags: review?(michael.l.comella)
(In reply to Sebastian Kaspari (:sebastian) from comment #12)
> Created attachment 8665023 [details]
> MozReview Request: Bug 1195692 - Replace AlertNotification with
> NotificationCompat.Builder based implementation. r?mcomella
> 
> Bug 1195692 - Replace AlertNotification with NotificationCompat.Builder
> based implementation. r?mcomella

Parts of this patch have been inspired by wesj's WIP patch in bug 893289. I've been testing this via demo pages for the Notification Web API. I've just been unable to follow the code path for setting progress. It almost feels like it's not used: I could not find any users of nsAlertsService::OnProgress. But that's the mysterious native world.
Comment on attachment 8665023 [details]
MozReview Request: Bug 1195692 - Replace AlertNotification with NotificationCompat.Builder based implementation. r?mcomella

https://reviewboard.mozilla.org/r/20123/#review18249

Looks good – don't forget to dupe bug 893289.
Attachment #8665023 - Flags: review?(michael.l.comella) → review+
This was the last piece for this bug.
Keywords: leave-open
Duplicate of this bug: 893289
https://hg.mozilla.org/mozilla-central/rev/4b20c11bc860
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 44
You need to log in before you can comment on or make changes to this bug.