Closed Bug 605801 Opened 9 years ago Closed 9 years ago

No restart notification when installing add-ons from AMO

Categories

(Firefox for Android Graveyard :: General, defect)

ARM
Android
defect
Not set

Tracking

(fennec2.0b2+)

VERIFIED FIXED
Tracking Status
fennec 2.0b2+ ---

People

(Reporter: ioana.chiorean, Assigned: wesj)

Details

Attachments

(1 file, 2 obsolete files)

Device: Motorola Droid 2
BuildID: Mozilla /5.0 (Maemo;Linux armv7l;rv:2.0b8pre)Gecko/20101020
Firefox/4.0b8pre Fennec /4.0b2pre 

Steps to reproduce:

1. Open Fennec
2. Go to AMO 
3. Install a recommended add-on (TwitterBar for example).

Expected results:
- A restart notification is displayed after the add-on is installed -> either present in the status bar of the device or a pop up in the right corner

Actual results:
- No notification is displayed.
Assignee: nobody → wjohnston
Yep, I can confirm this on builds:

Mozilla/5.0 (Maemo; Linux armv71; rv:2.0b8pre) Gecko/20101020 Namoroka/4.0b8pre Fennec/4.0b2pre

and

Mozilla/5.0 (Android; Linux armv71; rv:2.0b8pre) Gecko/20101020 Namoroka/4.0b8pre Fennec/4.0b2pre
tracking-fennec: --- → ?
Attached patch Fixes (obsolete) — Splinter Review
This should fix this. In my last fixes I moved the place where we were bailing out if no element existed for the addon. That was true any time we hadn't called _delayedInit here (i.e. if you hadn't looked at the Add-ons manager yet).

Here I'm not bailing out at all, but just moved all of the stuff requiring an element into its own block.

Tested this by modifying a nightly. Still building my daily build to test. Also started writing a test for this.
Attachment #484717 - Flags: review?(mark.finkle)
With this patch, tested and work:

1.) Visit AMO directly through urlbar, install addon
2.) Visit AMO through add-on manager page, install addon
3.) Install addon through add-on manager
Comment on attachment 484717 [details] [diff] [review]
Fixes


>   onInstallEnded: function(aInstall, aAddon) {
>     let element = ExtensionsView.getElementForAddon(aAddon.id);
>-    if (!element)
>-      return;
>+    this._updating = element && element.hasAttribute("updating");
>+    if (aAddon.pendingOperations & AddonManager.PENDING_INSTALL)
>+      ExtensionsView.showRestart(this._updating ? "update" : "normal");

Can we move the this._updating down into the | if (element) { | bock and use:

aAddon.pendingOperations & AddonManager.PENDING_UPGRADE

http://mxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/AddonManager.jsm#989

r+ but I'd like to smooth this out a little
Attachment #484717 - Flags: review?(mark.finkle) → review+
Attached patch Fix v2 (obsolete) — Splinter Review
Fixes after talking to Mossop about this. I've also got a few addons setup for testing this and addon upgrades/uninstalls:

https://dl.dropbox.com/u/72157/Testing/extensions/testInstallAndUpgrade.html
Attachment #484717 - Attachment is obsolete: true
Attachment #484889 - Flags: review+
Attached patch Fix v3.Splinter Review
Missed a call to of this._updating
Attachment #484889 - Attachment is obsolete: true
Attachment #484908 - Flags: review+
pushed:
http://hg.mozilla.org/mobile-browser/rev/1a81b76129f6
Status: NEW → RESOLVED
tracking-fennec: ? → 2.0b2+
Closed: 9 years ago
Resolution: --- → FIXED
verified FIXED on build:
Mozilla/5.0 (Android; Linux armv71; rv:2.0b8pre) Gecko/20101021 Namoroka/4.0b8pre Fennec/4.0b2pre
    
but follow-up filed in bug 606178. This is not happening on the n900 nightly.
Status: RESOLVED → VERIFIED
You need to log in before you can comment on or make changes to this bug.