394645 - show notification using nsIAlertService when extension updates are available.

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Mike Connor [:mconnor]]

We want to show some sort of notification that extensions are available, when they are found.  The closest thing we have to a global notification widget (bug 347585) right now is the alerts service, so we should utilize that for now.

This might be considered a stopgap, or the final solution, but either way it should represent an improvement over the current zero-notification UI, without interrupting the user.

Comment 1

[Dave Townsend [:mossop]]

Attachment: patch rev 1

This breaks out some of the special handling of the background update check into its own dedicated listener which keeps the ExtensionItemUpdater cleaner. Maintain a simple count of all the new updates detected and at the end of the update check display a notification.

If the notification is clicked on show the add-ons window with the updates pane, or just switch to it if the window is already visible. Also reset the start notification pref in this case.

Comment 2

[Robert Strong (they/them - no direct email)]

Comment on attachment 279361 [details] [diff] [review]
patch rev 1

Please run the strings by Madhava or beltzner.

>Index: toolkit/mozapps/extensions/src/nsExtensionManager.js.in
>...
>+  onUpdateEnded: function() {
>+    if (this._updateCount > 0 &&
>+        Components.classes["@mozilla.org/alerts-service;1"]) {
>+      var extensionStrings = BundleManager.getBundle(URI_EXTENSIONS_PROPERTIES);
>+      var title = extensionStrings.GetStringFromName("updateNotificationTitle");
>+      var text;
>+      if (this._updateCount > 1)
>+        text = extensionStrings.formatStringFromName("multipleUpdateNotificationText",
>+                                                     [BundleManager.appName, this._updateCount], 2);
>+      else
>+        text = extensionStrings.formatStringFromName("updateNotificationText",
>+                                                     [BundleManager.appName], 1);
>+
>+      try {
>+        var notifier = Components.classes["@mozilla.org/alerts-service;1"]
>+                                 .getService(Components.interfaces.nsIAlertsService);
>+        notifier.showAlertNotification(URI_GENERIC_ICON_XPINSTALL,
>+                                       title, text, true, "", this);
>+      }
>+      catch (e) {
>+        // The alerts service may fail on unsupported platforms
nit: why not just LOG the error?

r=me

Comment 3

[Madhava Enros [:madhava]]

Just a few quick message string tweaks:

+updateNotificationTitle=Updated add-ons found

Given the way we talk about the update process to users otherwise, I'd worry here that an "Updated add-on" sounds like one that's already been updated locally.  I'd go with:

Updates for Add-ons found

or

Add-on updates found



For the other two, I think I'd just change "an update to" to "an update for" for the same reasons:


+updateNotificationText=%S has found an update to 1 of your add-ons

%S has found an update for 1 of your add-ons


+multipleUpdateNotificationText=%S has found updates to %S of your add-ons

%S has found updates for %S of your add-ons

Comment 4

[Dave Townsend [:mossop]]

Attachment: patch for checkin

As before with string corrections, log message and Components.classes -> Cc etc. fixes to match trunk.

Comment 5

[Dave Townsend [:mossop]]

Checking in toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties;
/cvsroot/mozilla/toolkit/locales/en-US/chrome/mozapps/extensions/extensions.properties,v  <--  extensions.properties
new revision: 1.40; previous revision: 1.39
done
Checking in toolkit/mozapps/extensions/content/extensions.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/content/extensions.js,v  <--  extensions.js
new revision: 1.146; previous revision: 1.145
done
Checking in toolkit/mozapps/extensions/src/nsExtensionManager.js.in;
/cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v  <--  nsExtensionManager.js.in
new revision: 1.249; previous revision: 1.248
done

Comment 6

[Dave Townsend [:mossop]]

We no longer have this feature.