Closed Bug 489941 Opened 11 years ago Closed 10 years ago

Rework the Update Add-ons dialog

Categories

(Toolkit :: Add-ons Manager, defect)

x86
Windows XP
defect
Not set

Tracking

()

RESOLVED FIXED
Tracking Status
status1.9.2 --- beta1-fixed
fennec 1.0b1-wm+ ---

People

(Reporter: mfinkle, Assigned: mfinkle)

References

Details

(Keywords: uiwanted, Whiteboard: [fennec l10n])

Attachments

(3 files, 2 obsolete files)

When Fennec (or any other extension enabled Mozilla app) starts after an update, it checks to see if the installed add-ons are still compatible. It displays a dialog box if any are not compatible, allowing the user to check the updates to those add-ons.

This dialog causes problems on Windows Mobile because of size issues. It also creates a problem where the dialog does not take top-level focus, instead hiding behind other running applications. The user then has no way of knowing that Fennec has launched and is awaiting input about an add-on update check.

We should either fix the size and focus issues or remove the dialog altogether. Handling this situation inside Fennec might be easier (thinking UI here) and we could take actions based on prefs (just check or don't check, but don't bug me with a freaking dialog)
Blocks: 482816
Couldn't we use one of our lower-right-hand-corner alert boxes for this?
Yes, but this dialog appears before Fennec is even active/running, so we'd need to do some work to just make it skip past that point and let Fennec handle it.
tracking-fennec: --- → ?
tracking-fennec: ? → 1.0b1-wm+
Assignee: nobody → bugmail
IMO, the ideal solution here would be a patch that allows a default choice. Right now, we always show the update.xul window here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/src/nsExtensionManager.js.in#3382

Called from here:
http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/src/nsExtensionManager.js.in#3329

I think we could use a preference that tells the EM to just skip the update check (disabling add-ons that are not compatible) and then the application can tell the user about the situation when it loads.
I think for 1.9.3 we're going to be getting rid of this dialog everywhere. If you need it gone on 1.9.2 then I suspect the easiest option right now might be just to ifdef it off for mobile.
Attached patch patch (platform) (obsolete) — Splinter Review
Adds two preferences:
* Adds a preference to turn off/on the UI for incompatible add-ons found during an application update
* If the UI is off, the second preference will store the list of disabled add-on IDs
Assignee: bugmail → mark.finkle
Attachment #399570 - Flags: review?(dtownsend)
Attached patch patch (fennec) (obsolete) — Splinter Review
Depends on the platform patch
Attachment #400131 - Flags: review?(gavin.sharp)
Whiteboard: [fennec l10n]
Changed the strings slightly
Attachment #400131 - Attachment is obsolete: true
Attachment #400132 - Flags: review?(gavin.sharp)
Attachment #400131 - Flags: review?(gavin.sharp)
Attachment #400132 - Flags: review?(gavin.sharp) → review+
Instead of "One or more add-ons are no longer compatible and were disabled" how
about the shorter:

[# of add-ons] incompatible add-ons disabled
Comment on attachment 400132 [details] [diff] [review]
patch (fennec) - v2

>+alertAddonsDisabled=#1 incompatible add-on was disabled;#1 incompatible add-ons were disabled

... should have a localization note pointing to https://developer.mozilla.org/en/Localization_and_Plurals. We don't really have a better way to mark up plurals at this point in time.
(In reply to comment #9)

> ... should have a localization note pointing to
> https://developer.mozilla.org/en/Localization_and_Plurals. We don't really have
> a better way to mark up plurals at this point in time.

done
pushed the fennec part:
https://hg.mozilla.org/mobile-browser/rev/2808948efa5c

leaving open for the platform part
Attachment #399570 - Flags: review?(dtownsend) → review-
Comment on attachment 399570 [details] [diff] [review]
patch (platform)

Couple of tweaks here, would like to see it run by me again  but I expect it to be fine at that point.

>+const PREF_EM_DISABLED_ADDONS_LIST    = "extensions.disabledAddons";
>+const PREF_EM_DISABLED_ADDONS_UI      = "extensions.disabledAddons.showUI";

I don't really like this preference name, nor the constant name. How about extensions.showMismatchUI and PREF_EM_SHOW_MISMATCH_UI.

>-    // Always check for compatibility updates when upgrading if we have add-ons
>-    // that aren't managed by the application.
>-    if (!allAppManaged)
>-      this._showMismatchWindow(inactiveItemIDs);
>+    // Determine if we should check for compatibility updates when upgrading if
>+    // we have add-ons that aren't managed by the application.
>+    if (!allAppManaged) {

Let's give Firefox a win here too. We shouldn't show the UI if all add-ons are appManaged, or if none were disabled this time or if it is the first run.

>+      // Should we show a UI or just pass the list via a pref?
>+      if (getPref("getBoolPref", PREF_EM_DISABLED_ADDONS_UI, true)) {
>+        this._showMismatchWindow(inactiveItemIDs);
>+      }
>+      else {
>+        // Remember the list of add-ons that were disbaled this time around

"disabled"

>+        // unless this was a new profile.
>+        if (!gFirstRun && disabledAddons.length > 0)
>+          gPref.setCharPref(PREF_EM_DISABLED_ADDONS_LIST, disabledAddons.join(","));
>+        else
>+          gPref.clearUserPref(PREF_EM_DISABLED_ADDONS_LIST);
>+      }
>+    }
This should have all the requested changes.
Attachment #399570 - Attachment is obsolete: true
Attachment #400524 - Flags: review?(dtownsend)
Comment on attachment 400524 [details] [diff] [review]
patch (platform) - v2

Looks good, thanks
Attachment #400524 - Flags: review?(dtownsend) → review+
Update the pref name used in the fennec mobile.js prefs file. The pref was changed in the platform patch
Attachment #400800 - Flags: review?(gavin.sharp)
Assignee: mark.finkle → nobody
Component: General → Add-ons Manager
Product: Fennec → Toolkit
QA Contact: general → add-ons.manager
Attachment #400524 - Flags: approval1.9.2?
Assignee: nobody → mark.finkle
Attachment #400800 - Flags: review?(gavin.sharp) → review+
pushed platform:
http://hg.mozilla.org/mozilla-central/rev/3710814acfa9
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Attachment #400524 - Flags: approval1.9.2? → approval1.9.2+
You need to log in before you can comment on or make changes to this bug.