597868 - Addon update wizard on application upgrade not correctly installing addon updates

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Dave Garrett]

I noticed this while testing to make sure my own addon's alpha version was updatable to correctly. All Flagfox files used in the steps below are here:
https://addons.mozilla.org/en-US/firefox/addon/5791/versions/

You'll notice that Flagfox is installed correctly by of course seeing the flag icon show in the address bar. (or a different icon if not a web page) Flagfox 4.0.x and 4.1a1 also each have their own once ever shown first run pages.

STR:
1) New Firefox 3.6.10 profile
2) Install Flagfox 4.0.7; close all tabs and windows and exit Firefox
3) Load same profile in Firefox 4.0b6
4) You should see the update wizard and get to the "Incompatible Add-ons" dialog
5) Click "Check Now" and it will tell you Flagfox 4.0.8 is available
   (4.0.9 is pending review, so once that's public it'll be 4.0.9)
6) Click "Install Now" and then once it does, click "Finish"
7) Notice Flagfox isn't in the address bar
8) In the Error Console:
   Warning: WARN addons.manager: InstallListener threw exception when calling onDownloadStarted: ReferenceError: strings is not defined
   Warning: WARN addons.manager: InstallListener threw exception when calling onInstallStarted: ReferenceError: strings is not defined
9) about:addons has no icon for Flagfox; options/preferences and about fail
10) In the Error Console:
   No chrome package registered for chrome://flagfox/content/logo.png
   No chrome package registered for chrome://flagfox/content/options.xul
   No chrome package registered for chrome://flagfox/content/about.xul
11) Close Firefox 4.0b6 -> zombie process which you will need to kill
12) Retry loading Firefox 4.0b6 again, and it is still not loading Flagfox

If instead of letting the "Incompatible Add-ons" wizard do its thing and you refuse the update check, you can then load up 4.0b6, go to the Addons Manager, check for updates manually, install the correct Flagfox update, restart, and everything works fine. Thus, it's the upgrade *wizard* that's introducing some problem here.

Flagfox 4.0.x does not currently support above Firefox 4.0b6. I just created a Flagfox 4.1a1 to regain support after the extractionless XPI installation landing completely killed things. If you follow the STR above, however start by installing the (old and obsolete) Flagfox 4.0.8pre into Firefox 3.6, then switching to latest Minefield 4.0b7pre, the wizard will offer to update you instead to its AMO beta channel update of Flagfox 4.1a1 and get the same results. After some Minefield killing and a restart or two I can get it to finally load Flagfox correctly sometimes, but I couldn't get 4.0b6 to do that. Neither work on first go.

Further investigation of what's going on (using either versions' update routes) shows the following in the Firefox profile folder:

<profile>/extensions/{1018e4d6-728f-4b20-ad56-37578a4de76b}
<profile>/extensions/staged

These are two folders. "staged" is empty. The Flagfox folder, "{1018e4d6-728f-4b20-ad56-37578a4de76b}", contains the NEW version, which appears to have been extracted correctly.

"staged" deletes automatically after another start of Firefox 4, but the problem with the extension remains.

Deleting the extensions.* files in the profile directory of course resets things and upon next Firefox start Flagfox will load up fine. (note that Flagfox 4.0.8 is compatible with 4.0b6 by virtue of an AMO remote compatibility update which you'd have to check for to get it working; 4.1a1 and 4.0.9 list the needed compatibilities in their install.rdf files directly)

Again, just to repeat for clarity: if you refuse the wizard's help and update via about:addons yourself, everything works fine.

Tested under Kubuntu 10.04. UA strings for the versions referenced above:
Mozilla/5.0 (X11; Linux i686; rv:2.0b7pre) Gecko/20100919 Firefox/4.0b7pre
Mozilla/5.0 (X11; Linux i686; rv:2.0b6) Gecko/20100101 Firefox/4.0b6
Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9.2.10) Gecko/20100914 Firefox/3.6.10

This is of course a regression, as I can go from Firefox 3.5 with Flagfox 3.3.16 to Firefox 3.6 and the upgrade wizard will successfully update it to Flagfox 4.0.8.

Comment 1

[Dave Garrett]

Just tested in a VM with Windows XP SP3. Reproducible there too. I even got a full freeze/hang on clicking the Flagfox options button in about:addons as apposed to a zombie process after-the-fact. Bumping severity to critical because of the total Firefox stall this can cause. (process kill needed) This bug also may be considered dataloss, depending on how dataloss is precisely defined.

Comment 2

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Have also seen reports for it from input.

Comment 3

[Dave Townsend [:mossop]]

I think we're not rewriting the extensions.ini file after the dialog runs through. This would leave the extensions installed bu the platform would never load them.

Comment 4

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Some better testing reveals this to be the case. The testcase now verifies that the add-ons list is correct and also adds an add-on that requires an update check to become compatible and verifies that that remains in the extensions list.

Comment 5

[Dave Townsend [:mossop]]

Attachment: patch rev 1

Forgot to add the new files.

Comment 6

[Robert Strong (they/them - no direct email)]

Comment on attachment 478447 [details] [diff] [review]
patch rev 1

well sleuthed!

Comment 7

[Dave Garrett]

Not possible to land for beta 7? Some people will hit it on beta 7 update.

(In reply to comment #3)
> I think we're not rewriting the extensions.ini file after the dialog runs
> through. This would leave the extensions installed bu the platform would never
> load them.

Is it practical to add a sanity check of some kind to detect a bogus or at least outdated extensions.ini file from any cause and regenerate it? (practical meaning detection has no significant impact on startup time) Profiles do get borked on occasion, so it would be nice to have a failsafe to correct this and any other broken extensions.ini file if it's doable.

Comment 8

[Dave Townsend [:mossop]]

(In reply to comment #7)
> Not possible to land for beta 7? Some people will hit it on beta 7 update.

I'll speak to the drivers but normally only b7 blockers are allowed to land right now.

> (In reply to comment #3)
> > I think we're not rewriting the extensions.ini file after the dialog runs
> > through. This would leave the extensions installed bu the platform would never
> > load them.
> 
> Is it practical to add a sanity check of some kind to detect a bogus or at
> least outdated extensions.ini file from any cause and regenerate it? (practical
> meaning detection has no significant impact on startup time) Profiles do get
> borked on occasion, so it would be nice to have a failsafe to correct this and
> any other broken extensions.ini file if it's doable.

Not really without loading the database on every startup and rewriting extensions.ini then, and that would certainly impact startup time.

Comment 9

[Dave Garrett]

(In reply to comment #8)
> Not really without loading the database on every startup and rewriting
> extensions.ini then, and that would certainly impact startup time.

I was thinking something simpler like attempting to check for an old modified time on the ini vs. anything in the extensions dir.

Comment 10

[Dave Townsend [:mossop]]

(In reply to comment #9)
> (In reply to comment #8)
> > Not really without loading the database on every startup and rewriting
> > extensions.ini then, and that would certainly impact startup time.
> 
> I was thinking something simpler like attempting to check for an old modified
> time on the ini vs. anything in the extensions dir.

That could work though it would yield a bunch of false positives, it also would not have helped in this particular case.

Comment 11

[Dave Garrett]

(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > Not really without loading the database on every startup and rewriting
> > > extensions.ini then, and that would certainly impact startup time.
> > 
> > I was thinking something simpler like attempting to check for an old modified
> > time on the ini vs. anything in the extensions dir.
> 
> That could work though it would yield a bunch of false positives, it also would
> not have helped in this particular case.

I see, the file is modified, just modified wrong.

Only other quick check I can think of might be to enumerate the dir names in /extensions and make sure they're all counted in the ini when loading the ini. Though, my guess is one of the reasons the ini exists at all is avoid the cost of exactly that.

The other fallback possibility would be to catch any errors on loading everything for about:addons and rebuild anything as needed there, rather than on startup. Would be nice if loading about:addons could auto-fix some corruption problems automatically. Worth filing a bug for this?

Comment 12

[Dave Townsend [:mossop]]

(In reply to comment #11)
> (In reply to comment #10)
> > (In reply to comment #9)
> > > (In reply to comment #8)
> > > > Not really without loading the database on every startup and rewriting
> > > > extensions.ini then, and that would certainly impact startup time.
> > > 
> > > I was thinking something simpler like attempting to check for an old modified
> > > time on the ini vs. anything in the extensions dir.
> > 
> > That could work though it would yield a bunch of false positives, it also would
> > not have helped in this particular case.
> 
> I see, the file is modified, just modified wrong.
> 
> Only other quick check I can think of might be to enumerate the dir names in
> /extensions and make sure they're all counted in the ini when loading the ini.
> Though, my guess is one of the reasons the ini exists at all is avoid the cost
> of exactly that.

Actually the point is that we often do not want to include installed extensions in extensions.ini, when they are disabled for example.

> The other fallback possibility would be to catch any errors on loading
> everything for about:addons and rebuild anything as needed there, rather than
> on startup. Would be nice if loading about:addons could auto-fix some
> corruption problems automatically. Worth filing a bug for this?

It's actually difficult for the frontend UI to figure out there is a problem going on. It might be worth having a bug on file to figure out some way to do something with this though.

Comment 13

[Dave Garrett]

Filed bug 599572 for the corruption check/repair idea.

Comment 14

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Dave, I tried to follow the steps in comment 0 but with 4b6 the update wizard doesn't display the page with available add-on updates when extensions.logging.enabled is set to true. It works with Minefield but in that case we do not have compatible add-ons. Is that a bug for release builds only? It kinda makes it hard to debug add-on update issues.

Comment 15

[Dave Garrett]

(In reply to comment #14)
To clarify from comment 0, I can reproduce with either of two paths:
a) Firefox 3.6.10 -> 4.0b6 with Flagfox 4.0.7 -> 4.0.8 (now 4.0.9)
b) Firefox 3.6.10 -> 4.0b7pre with Flagfox 4.0.8pre -> 4.1a1

These are the two available update channels from the Mozilla Addons site. Route 'a' is the normal stable update channel and route 'b' is the "beta channel", which also handles alphas and any properly marked prereleases. (including the "pre" and the "a" in the versions in route 'b')

(In comment #0)
> Flagfox 4.0.x does not currently support above Firefox 4.0b6. I just created a
> Flagfox 4.1a1 to regain support after the extractionless XPI installation
> landing completely killed things. If you follow the STR above, however start by
> installing the (old and obsolete) Flagfox 4.0.8pre into Firefox 3.6, then
> switching to latest Minefield 4.0b7pre, the wizard will offer to update you
> instead to its AMO beta channel update of Flagfox 4.1a1 and get the same
> results.

> (note that Flagfox 4.0.8 is compatible with 4.0b6 by virtue of an AMO remote
> compatibility update which you'd have to check for to get it working; 4.1a1
> and 4.0.9 list the needed compatibilities in their install.rdf files directly)

Comment 16

[Dave Townsend [:mossop]]

I would like to land this for beta 7. Although I think we could ship beta 7 without it it I think it would be beneficial to include it. This code is only used when updating to a new version of the application so we essentially only get a wide scale user testing of it whenever a new beta goes out. Not having it will also potentially harm the experience of the additional less technical beta users that we are planning to find for b7 as it will leave them with add-ons appearing to be in a broken state until they manually make a change and restart.

Comment 17

[Dave Garrett]

Beta 7 will also need more compatibility updates than prior betas.

Comment 18

[Dave Townsend [:mossop]]

Landed: http://hg.mozilla.org/mozilla-central/rev/bd0966d9ceed

Comment 19

[Henrik Skupin [:whimboo][⌚️UTC+2]]

(In reply to comment #15)
> b) Firefox 3.6.10 -> 4.0b7pre with Flagfox 4.0.8pre -> 4.1a1

While trying to verify this fix I see a major issue on OS X after the new version has been downloaded. I'm not able to start Minefield anymore. That also happens with the latest nightly build. Deleting the sqlite file solved this problem, but copying back the former state of the profile doesn't end-up in the same situation. I have to redo all the steps again.

Does anyone see the same?

Comment 20

[Dave Garrett]

(In reply to comment #19)
I just retested Firefox 3.6.10 -> 4.0b8pre with Flagfox 4.0.8pre -> 4.1a2 and had no problems. (new profile under Linux with latest Minefield) Minefield works fine and restarted fine. Are you only seeing an issue on Mac or only one profile?

Comment 21

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Ok, as it turns out, it has nothing to do with the Add-ons Manager. I simply ran into a new regression. Filed as bug 603699.

Comment 22

[Henrik Skupin [:whimboo][⌚️UTC+2]]

Finally verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b12pre) Gecko/20110209 Firefox/4.0b12pre ID:20110209030359