433371 - Ubuntu 8.04's firefox install breaks 3.0pre's addons

Status: VERIFIED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[Vladimir Vukicevic [:vlad] [:vladv] (needinfo me, slow to respond)]

Ubuntu firefox-3.0~b5+nobinonly-0ubuntu3
Nightly 20080151204

Create a profile using the ubuntu firefox:

/usr/bin/firefox -createProfile mold -no-remote
/usr/bin/firefox -P mold -no-remote
.. quit ..

Open nightly firefox with the created profile:

~/firefox/firefox -P mold -no-remote

12:03 < vlad> Mossop: so I created a profile with ubuntu's
12:03 < vlad> I open that profile in latest trunk
12:04 < vlad> in addons mgr, I see "Ubuntu Firefox Modifications 0.5"  "This 
              add-on will be uninstalled when Minefield is restarted."
12:04 < vlad> (I didn't do anything to get it there)
12:04 < vlad> clicking restart restarts, and gets me back to the same state
12:04 < vlad> if I try to install any addon, I get an error
12:05 < vlad> Minefield could not install the file at ... because: Unexpected 
              installation error
12:05 < vlad> Error: installLocation is null
12:05 < vlad> Source File: 
              file:///home/vladimir/firefox/components/nsExtensionManager.js
12:05 < vlad> Line: 7971
12:05 < vlad> the (new) extension is half-installed in the profile dir
12:06 < vlad> is there an existing bug I should be referencing?
12:06 < vlad> oh, in error console, first entry is 'oldLocation is null' Line 
              7867
12:06 < vlad> then a bunch of No chrome package registered for chrome://ubufox

I can "fix" this profile by editing prefs.js and deleting the extensions.enabledItems pref.

Comment 1

[Dave Townsend [:mossop]]

I think we should consider whether to block on this. When switching from a custom build to the official build the effect is that new add-ons cannot be installed and all existing add-ons do not work. However it does appear restricted to Ubuntu, SuSE and Fedora users should not be affected. I have not looked into other distributions. Additionally users can work around the issue by deleting the extensions.rdf file from their profile folder. So perhaps the impact is small enough that we could defer to a point release.

This is really a missed case from bug 356370. In Ubuntu's customised Firefox the default theme is in a special install location (gre-global). When the official Firefox runs it knows nothing of this install location so it must clear out the bad data from the datasource. However before the code that does this kicks in a different copy of the default theme is detected in the Firefox application directory. The process of installing this hits two points where it tries to access the unknown install location and throws an error meaning the bad entries are never cleaned up properly.

I do have a simple and low risk patch practically ready that fixes the issue and includes additional cases for the unit test for bug 356370 to properly test this scenario.

Comment 2

[Mike Beltzner [:beltzner, not reading bugmail]]

Dave, any idea if we can get Ubuntu to fix this through an update on their side when they move to Firefox 3 final?

I don't think this blocks final, but yes, good for point release. Get the patch up and reviewed ASAP, and mark it approval1.9? when it's ready to go. If we do an RC2 we'll take it, if not, we'll get it in 3.0.1 :)

Comment 3

[Dave Townsend [:mossop]]

(In reply to comment #2)
> Dave, any idea if we can get Ubuntu to fix this through an update on their side
> when they move to Firefox 3 final?

It is not something Ubuntu could fix (short of completely changing their packaging structure for XULRunner based applications). However when they update to Firefox 3 final they will still be using the same customisations so it will not affect users. It only affects people switching from the Ubuntu customised Firefox to the Official Mozilla Firefox

Comment 4

[Dave Townsend [:mossop]]

Attachment: patch rev 1

So there two failures that we experience when having a theme registered in an unknown install location and a new copy of that theme detected in a new install location.

First is specific to themes, when building the new active list we call getItemForID which attempts to build data from the in-datasource (invalid) instance of the theme, which tries to query the theme's icon and preview image which fails. That is a simple fix.

Secondly when updating the visible list the old instance of the add-on causes an |oldLocation is null| failure. We can just always update the visible list if the oldLocation is unknown.

The unit test is just more easily done by adding this case to the unit test for bug 356370. It adds a theme in an unknown location and then copies in the install.rdf for a matching theme to the profile and allows it to be detected on startup. This fails on both the above issues without the patch and passes with.

Comment 5

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

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

nice!

Comment 6

[Dave Townsend [:mossop]]

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

Seeking approval to land. The actual code changes are minor and would have no impact on normal operation, only correcting error cases. It has unit test coverage.

Comment 8

[Dave Townsend [:mossop]]

We ought to note this in the release notes for RC1 I think

Comment 11

[Mike Schroepfer]

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

a+ please land on CVS trunk.

Comment 12

[Dave Townsend [:mossop]]

Landed on CVS trunk. Leaving open till I can figure out what to do about hg

Checking in toolkit/mozapps/extensions/src/nsExtensionManager.js.in;
/cvsroot/mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in,v  <--  nsExtensionManager.js.in
new revision: 1.288; previous revision: 1.287
done
Checking in toolkit/mozapps/extensions/test/unit/test_bug356370.js;
/cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/test_bug356370.js,v  <--  test_bug356370.js
new revision: 1.2; previous revision: 1.1
done
Checking in toolkit/mozapps/extensions/test/unit/data/test_bug356370.rdf;
/cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/data/test_bug356370.rdf,v  <--  test_bug356370.rdf
new revision: 1.2; previous revision: 1.1
done
RCS file: /cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/data/test_bug356370_4.rdf,v
done
Checking in toolkit/mozapps/extensions/test/unit/data/test_bug356370_4.rdf;
/cvsroot/mozilla/toolkit/mozapps/extensions/test/unit/data/test_bug356370_4.rdf,v  <--  test_bug356370_4.rdf
initial revision: 1.1
done

Comment 13

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

Dave, shouldn't it get Target Milestone: Firefox 3.1? It's only checked in on trunk right now. Will we have a new keyword for fixed bugs on 3.0 branch?

Comment 14

[Mike Shaver (:shaver emeritus)]

If we spin RC2, this will be in Firefox 3.0.

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

We'll be merging back to HG (bug 433426) so we don't need to explicitly track these landings at this time.

Comment 16

[Tony Chung [:tchung]]

clint, tomcat, do either of you have ubuntu 8.0.4?  if so, can you verify this bug on rc2?  Thanks.

Comment 17

[cmtalbert]

(In reply to comment #16)
> clint, tomcat, do either of you have ubuntu 8.0.4?  if so, can you verify this
> bug on rc2?  Thanks.
> 
I take this :-D

Comment 18

[cmtalbert]

verified on RC2 build 2: Mozilla/5.0 (X11; U; Linux i686; en-US; rv:1.9) Gecko/2008052912 Firefox/3.0

with Ubuntu 8.04.