Add support for showing and hiding restart doorhangers in Add-on Manager

VERIFIED FIXED in Firefox 13

Status

()

Firefox for Android
General
P3
normal
VERIFIED FIXED
6 years ago
9 months ago

People

(Reporter: mfinkle, Assigned: mbrubeck)

Tracking

unspecified
Firefox 14
ARM
Android
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(firefox13 fixed, firefox14 verified, blocking-fennec1.0 +, fennec11+)

Details

Attachments

(1 attachment)

When enabling, disabling and uninstalling non-restartless add-ons in the Add-on Manager, we need to manage the "restart" doorhanger. We need to show and hide it dynamically as the user manipulates the add-on.

Updated

6 years ago
OS: Linux → Android
Hardware: x86 → ARM
(Reporter)

Updated

6 years ago
Assignee: nobody → mark.finkle
Priority: -- → P3
tracking-fennec: --- → 11+

Updated

5 years ago
blocking-fennec1.0: --- → ?
blocking-fennec1.0: ? → +
(Reporter)

Updated

5 years ago
Assignee: mark.finkle → mbrubeck
(Assignee)

Updated

5 years ago
Blocks: 738429
(Assignee)

Comment 1

5 years ago
Created attachment 608918 [details] [diff] [review]
patch

In addition to implementing showRestart and hideRestart, this patch also:

- does some minor refactoring in aboutAddons.js to avoid repeated "detailItem.addon", and

- fixes a simple bug that broke NativeWindow.doorhanger.hide.
Attachment #608918 - Flags: review?(margaret.leibovic)

Comment 2

5 years ago
Comment on attachment 608918 [details] [diff] [review]
patch

>diff --git a/mobile/android/chrome/content/aboutAddons.js b/mobile/android/chrome/content/aboutAddons.js

>   setEnabled: function setEnabled(aValue, aAddon) {
>     let detailItem = document.querySelector("#addons-details > .addon-item");
>     let addon = aAddon || detailItem.addon;
>     if (!addon)
>       return;
> 
>+    let listItem = this._getElementForAddon(addon.id);

Can listItem be null? ...

>       if ((addon.pendingOperations & AddonManager.PENDING_ENABLE) ||
>           (addon.pendingOperations & AddonManager.PENDING_DISABLE)) {
>         this.showRestart();
>-      } else if (addon == detailItem.addon &&
>-            detailItem.getAttribute("opType") == "needs-disable" ||
>-            detailItem.getAttribute("opType") == "needs-enable") {
>+      } else if (/needs-(enable|disable)/.test(listItem.getAttribute("opType"))) {

... if so, this could cause an error...

>     // Sync to the list item
>-    let listItem = this._getElementForAddon(addon.id);
>     if (listItem) {

... if not, we could get rid of this check :)

>     } else {
>-      detailItem.addon.uninstall();
>-      let opType = this._getOpTypeForOperations(detailItem.addon.pendingOperations);
>+      addon.uninstall();
>+      let opType = this._getOpTypeForOperations(addon.pendingOperations);

Nit: While you're cleaning this up, you could stick this declaration inside the if statement below it, since it's not used elsewhere.
Attachment #608918 - Flags: review?(margaret.leibovic) → review+
(Assignee)

Updated

5 years ago
Duplicate of this bug: 738429
(Assignee)

Comment 4

5 years ago
Landed with review comments addressed:
https://hg.mozilla.org/integration/mozilla-inbound/rev/26df3b9b439f
Status: NEW → ASSIGNED
Target Milestone: --- → Firefox 14
https://hg.mozilla.org/mozilla-central/rev/26df3b9b439f
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
(Assignee)

Comment 6

5 years ago
Comment on attachment 608918 [details] [diff] [review]
patch

[Approval Request Comment]
User impact if declined: Add-on manager does not prompt to restart when a restart is necessary.

Testing completed (on m-c, etc.): Landed on m-c March 27.

Risk to taking this patch (and alternatives if risky): Patch is mobile-only and low-risk.

String changes made by this patch: None.
Attachment #608918 - Flags: approval-mozilla-aurora?

Comment 7

5 years ago
Verified/fixed on:
Nightly Fennec 14.0a1 (2012-03-29)
Device: Samsung Nexus S
OS: Android 2.3.6
Status: RESOLVED → VERIFIED
status-firefox14: --- → verified

Comment 8

5 years ago
Comment on attachment 608918 [details] [diff] [review]
patch

[Triage Comment]
Low risk, mobile-only Fennec blocker. Approved for Aurora 13.
Attachment #608918 - Flags: approval-mozilla-aurora? → approval-mozilla-aurora+
(Assignee)

Comment 9

5 years ago
https://hg.mozilla.org/releases/mozilla-aurora/rev/63c5274f27cb
status-firefox13: --- → fixed
You need to log in before you can comment on or make changes to this bug.