Created attachment 597346 [details]
Screenshot with the add-on
This is happening on all platforms.
Steps to reproduce:
1. Start Firefox 11 beta with a new, clean profile.
2. Change extensions.update.url to addons-dev.allizom.org and set the extensions.update.interval pref to 10
3. Restart Firefox for the changes to take effect and open Add-ons Manager
4. Browse for a while with the Add-ons Manager open.
The default add-on should update (ex:Feedback)
In the time that the Feedback add-on is updating, a Mozilla Hotfix update appears and it's disabled with no working buttons (see screenshot)
*Note: the add-on disappears when restarting the add-ons manager or the browser.
Blair, would you have time to take a look at this? I think the download entry for the add-on is getting stuck somehow
Created attachment 597448 [details]
screenshot of hotfix download failure
Alternative symptom with an old Firefox profile: the Hotfix is shown there, and when manually updating add-ons with "Check for Updates" from the Add-ons Manager, Firefox Beta 11 tries to update also the Hotfix and fails.
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0) Gecko/20100101 Firefox/11.0 ID:20120208012847
(In reply to hannu.nyman from comment #2)
> Created attachment 597448 [details]
> screenshot of hotfix download failure
> Alternative symptom with an old Firefox profile: the Hotfix is shown there,
> and when manually updating add-ons with "Check for Updates" from the Add-ons
> Manager, Firefox Beta 11 tries to update also the Hotfix and fails.
> Mozilla/5.0 (Windows NT 6.1; WOW64; rv:11.0) Gecko/20100101 Firefox/11.0
This is a different bug, we shouldn't be checking for hotfix updates in response to that button at all, or at least if we do then we need to add special logic to handle it.
Seems to fail removing the file from /trash/ when uninstalling (it uninstalls itself immediately on startup). Which is fine, but interesting.
But it then throws in AddonWrapper.hasResource(). Since that's being called after uninstall (after the file is moved to /trash/), the source bundle no longer exists, so calling nsIFile.isDirectory() on that throws.
Not sure why removing that file is failing, or why hasResource is being called after uninstall. I suspect it's just a timing issue. Will investigate further tomorrow.
My guess would be the zip file is locked open accessing the icon in there.
UI was breaking for two reasons:
* Addon.hasResource() was being called after uninstall and throwing an exception, but everything assumes that function will never throw
* startup() in bootstrap.js is called before onInstalled/onInstallEnded are sent. Since the addon uninstalls itself in startup(), onUninstalled could be sent before onInstalled/onInstallEnded.
I have a working patch - just gotta write some tests.
Created attachment 599482 [details] [diff] [review]
This API change concerns me a little. The intention was that after onInstalled is called the add-on is running. I guess that's not such a big deal but I wonder if we still need a way to get that information (to save you having to wait for a pref to be set in the tests f.e.).
There are some 370 add-ons on AMO that call addAddonListener. Thankfully the majority of them are Brand Thunder based so it might be interesting to verify that nothing breaks there.
Yea, I pondered that. But I think the only reasons that would matter would be:
* An addon depends on another addon (rare, and listening for onInstalled would be wrong anyway)
* An addon is handling the install of itself wrong (using onInstalled instead of the bootstrap install function).
I went through every addon on AMO that listens for onInstalled. Most of those cases are an empty function (useful, huh?). The other cases, I couldn't see anything that could break (just looking at the code).
I did wonder about firing onEnabled, which seems semantically correct. Especially given that non-restartless extensions can be installed disabled (by disabling when onInstalling fires). I was kinda scared by the number of tests that could potentially break, but looking at them, it may not be that many after all.
Comment on attachment 599482 [details] [diff] [review]
Let's go with this. Could you blog about the change, maybe also mention it to jorge and we'll see what kind of response we get?
(Also emailed Jorge.)
I had meant to land this before the merge, but then got sick. This this something we want on Fx13?
(In reply to Blair McBride (:Unfocused) from comment #12)
> I had meant to land this before the merge, but then got sick. This this
> something we want on Fx13?
I don't think we need to push it out that fast, it's a small UI issue that only happens if the user has the add-ons manager open at the time of install.
(In reply to Dave Townsend (:Mossop) from comment #13)
> I don't think we need to push it out that fast, it's a small UI issue that
> only happens if the user has the add-ons manager open at the time of install.
Agreed - we do have plans to use the hotfix in FF13 to enable an about:home pref mid-cycle (related to the Mozilla Marketplace), but we'll likely just flip the pref and have it uninstall itself immediately afterwards.
(In reply to Alex Keybl [:akeybl] from comment #15)
> Agreed - we do have plans to use the hotfix in FF13 to enable an about:home
> pref mid-cycle (related to the Mozilla Marketplace), but we'll likely just
> flip the pref and have it uninstall itself immediately afterwards.
If the Add-ons Manager is open when that happens, that will trigger this (minor) bug. But as an easy work-around just delay uninstalling by, say, 1 second.