Closed Bug 394645 Opened 12 years ago Closed 12 years ago

show notification using nsIAlertService when extension updates are available.

Categories

(Toolkit :: Add-ons Manager, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla1.9alpha8

People

(Reporter: mconnor, Assigned: mossop)

Details

Attachments

(1 file, 1 obsolete file)

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.
Attached patch patch rev 1 (obsolete) — Splinter Review
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.
Attachment #279361 - Flags: review?(robert.bugzilla)
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
Attachment #279361 - Flags: review?(robert.bugzilla) → review+
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
As before with string corrections, log message and Components.classes -> Cc etc. fixes to match trunk.
Attachment #279361 - Attachment is obsolete: true
Attachment #279615 - Flags: review+
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
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → Firefox 3 M8
OS: Mac OS X → All
Hardware: PC → All
Flags: in-testsuite?
Product: Firefox → Toolkit
We no longer have this feature.
Flags: in-testsuite? → in-testsuite-
Flags: in-litmus-
You need to log in before you can comment on or make changes to this bug.