619518 - Implement a toaster popup service

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Wesley Johnston (:wesj)]

There are a few cases where we would like to show a notification to give the user feedback, but there really isn't anything for them to do. In that case, it might be better just to show a toaster popup. And easy way to do this without creating a whole new interface would be to just do this if there is no callback attached to the notification.

Comment 1

[Mark Finkle (:mfinkle) (use needinfo?)]

Would we be better served by a service for toaster alerts? I think we would. Overloading one service to "do one or the other" seems confusing. I like being explicit.

We could use a new interface, like nsIAlertPopup or nsIAlertToaster, but I think we want to use the exact same method as the current nsIAlertsService. Therefore, we might only need a new implementation for nsIAlertsService.

for the default alerts service:

let alertsService = Cc["@mozilla.org/alerts-service;1"].getService(Ci.nsIAlertsService);

for the toaster alerts:

let alertsService = Cc["@mozilla.org/toaster-alerts-service;1"].getService(Ci.nsIAlertsService);

Comment 2

[[:fabrice] Fabrice Desré]

Attachment: wip

This patch adds a new toaster alert service (copying from the JS alert service) and uses it in a few places.

Wes, can you check I didn't miss any "non-callback" usage?

Comment 3

[Wesley Johnston (:wesj)]

Changing the title of this bug because it is completely wrong at the point. Based on talking to mfinkle at the work week, we aren't going to worry about callbacks. Just use this service where we want.

The majority use case we talked about was the extensions service, which can spam the notification service a lot. I think we wanted to change the notifications for addon download or install errors, and the download progress to a simpler initial onDownloadStarted toaster.

Comment 4

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 502878 [details] [diff] [review]
wip

good general direction, but I think we can re-use the AlertsService.js file and just add a second manifest "contract" pointing to the AlertsService guid.

We would need to remove the ifndef ANDROID in the components/Makefile.in for AlertsService.js and instead add it to the manifest so "@mozilla.org/system-alerts-service;1" contract is not included for Android. 

Make sense?

The 3 spots you are using the toaster alert seems OK. We might drop the "aCallback" check in HelperAppDialog.js

Comment 5

[Mark Finkle (:mfinkle) (use needinfo?)]

Comment on attachment 502878 [details] [diff] [review]
wip

>-    let notifier = Cc["@mozilla.org/alerts-service;1"].getService(Ci.nsIAlertsService);
>+    let notifier = aCallback ? Cc["@mozilla.org/alerts-service;1"].getService(Ci.nsIAlertsService) : Cc["@mozilla.org/toaster-alerts-service;1"].getService(Ci.nsIAlertsService);

How about:

let cid = aCallback ? "@mozilla.org/alerts-service;1" : "@mozilla.org/toaster-alerts-service;1";
let notifier = Cc[cid].getService(Ci.nsIAlertsService);

Comment 6

[Mark Finkle (:mfinkle) (use needinfo?)]

Let's move forward

Comment 7

[[:fabrice] Fabrice Desré]

Attachment: patch

Implements Mark's comments. Looks to work fine on linux/desktop and android.

Comment 8

[[:fabrice] Fabrice Desré]

pushed: http://hg.mozilla.org/mobile-browser/rev/128f90bc0b9c

Comment 9

[Anna (Waverley)]

VERIFIED FIXED on: 

Build ID: Mozilla /5.0 (Android;Linux armv7l;rv:2.2a1pre) Gecko/20110406 Firefox/4.2a1pre Fennec /4.1a1pre

Device: Motorola Droid  2 (Android 2.2)