Rework the Update Add-ons dialog

RESOLVED FIXED

Status

()

Toolkit
Add-ons Manager
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: mfinkle, Assigned: mfinkle)

Tracking

({uiwanted})

Trunk
x86
Windows XP
uiwanted
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(status1.9.2 beta1-fixed, fennec1.0b1-wm+)

Details

(Whiteboard: [fennec l10n])

Attachments

(3 attachments, 2 obsolete attachments)

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)
(Assignee)

Updated

9 years ago
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.
(Assignee)

Updated

8 years ago
tracking-fennec: --- → ?

Updated

8 years ago
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.
Created attachment 399570 [details] [diff] [review]
patch (platform)

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)
Created attachment 400131 [details] [diff] [review]
patch (fennec)

Depends on the platform patch
Attachment #400131 - Flags: review?(gavin.sharp)
(Assignee)

Updated

8 years ago
Whiteboard: [fennec l10n]
Created attachment 400132 [details] [diff] [review]
patch (fennec) - v2

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 9

8 years ago
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);
>+      }
>+    }
Created attachment 400524 [details] [diff] [review]
patch (platform) - v2

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+
Created attachment 400800 [details] [diff] [review]
patch (fennec) tweaks a prefname

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)

Updated

8 years ago
Assignee: mark.finkle → nobody
Component: General → Add-ons Manager
Product: Fennec → Toolkit
QA Contact: general → add-ons.manager
(Assignee)

Updated

8 years ago
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
Last Resolved: 8 years ago
Resolution: --- → FIXED
pushed last fennec tweak:
https://hg.mozilla.org/mobile-browser/rev/965ffe4133e4

Updated

8 years ago
Attachment #400524 - Flags: approval1.9.2? → approval1.9.2+
(Assignee)

Updated

8 years ago
status1.9.2: --- → beta1-fixed
I highly suspect this caused bug 542391
Depends on: 542391
You need to log in before you can comment on or make changes to this bug.