296578 - When upgrading from a 1.0 profile themes are not listed in the Theme Manager

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

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

The changes introduced in bug 286034 include extracting a theme's install.rdf to
the root of its install directory when it is installed and bug 291946 added
extracting the theme's icon and preview images to the root of its install
directory. For an installed theme to register when upgrading the app it will
need to have these same operations performed on the theme along with the
generation of a chrome.manifest.

Comment 1

[Chris Blore]

Obviously a necessary feature for when we begin the transition to Firefox 1.1. 

Requesting blocking-aviary1.1.

Comment 2

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

I found another serious problem while writing this patch that will more than
likely have to be taken into account with this patch. When upgrading from 1.0 to
1.1 the version check will not happen since the compatibility.ini is updated
prior to the extensions datasource being created. I think the cleanest solution
is to go ahead and create the datasource, chrome.manifests, etc. prior to the
version check.

Comment 3

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

Attachment: patch in progress

I believe this covers most of the issues with upgrading a profile. I am
probably going to add a check of the 1.0 extensions.rdf for updated target app
minVersion and maxVersion when the new extensions.rdf doesn't exist yet and
thereby avoid checking for updates on extensions that have already been set as
compatible based on the extension's update RDF.

Comment 4

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

Comment on attachment 185439 [details] [diff] [review]
patch in progress

Benjamin - this patch will upgrade Pre 1.1 installed themes to the new
structure and then if there have been any changes it finishes the install.
Without this last bit getIncompatibleItemList will return zero items since
there aren't any installed extensions in the extensions datasource at this
stage when upgrading from a 1.0 profile.

I haven't convinced myself that it should also read the 1.0 profile's
extensions.rdf in order to bring over the compatibility info. If it is decided
this is a good thing I would prefer to patch it in another bug and I am almost
finished with a patch that can provide this while also migrating the extensions
disabled status. If you would prefer I include that in this patch just minus
this.

Comment 5

[Benjamin Smedberg]

If you decide to try and read the 1.0 extensions.rdf, please also consider
migrating the "disabled" arcs from that manifest, so that if users have manually
disabled an extension in 1.0 it will also be disabled in 1.1. I have no
particular opinion one way or the other about reading the old RDF, but I
certainly don't consider it a 1.1-blocker.

Comment 6

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

I am going to go ahead and add it to this patch since if nothing else it will
reduce the number of extensions shown as incompatible when upgrading to 1.1
which can be very disconcerting when upgrading. I already have the additional
code working including the migration of the disabled arcs and all that's left is
making sure the logic for when to do this makes sense.

Comment 7

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

Attachment: better patch

This adds migration of the disabled attribute if disabled and compatibility
info when the extension or theme being installed from a 1.0 profile if it is
not compatible and the compatibility info in the old extensions datasource will
make it compatible when upgrading. I still need to work on the comments and
some additional cleanup.

Comment 8

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

Attachment: patch

This adds a new function named _upgradeFromV10 that is used for upgrading a 1.0
profile and performs the following:
1) removes the abandoned default theme directory from the profile (moved from
checkForMismatches).
2) prepares themes installed into a version 1.0 for installation.
3) initiates an install to populate the new extensions datasource.
4) migrates the disabled attribute from the old datasource.
5) migrates the app compatibility info from the old datasource.

This changes _getThemeJARURL to _getThemeImageURL as we discussed briefly.

This also fixes:
supports-PRBool that was mistyped as supports-PRUint8 in the update landing
incorrect call to updateTargetAppInfo
part of a patch that was over-written by the update landing where aData is now
checked for "update-check-success"
a reference to the old dtd file in update.xml that wasn't changed when it was
moved into extensions with the update landing

Comment 9

[Benjamin Smedberg]

Comment on attachment 185809 [details] [diff] [review]
patch

I only have one issue with this patch: we should not do the _upgradeFromV10
check if we already have a "new" extensions.rdf file. It is possible that
somebody runs 1.0, 1.1, 1.0, 1.1, which would trigger the currAppVersion !=
lastAppVersion check but should not trigger _upgradeFromV10

Comment 10

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

Attachment: patch

Now it only calls it if the extensions.rdf in the profile dir does not exist.

Thanks!

Comment 11

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

Fixed on trunk

Comment 12

[timeless]

Comment on attachment 185818 [details] [diff] [review]
patch

mozilla/toolkit/mozapps/extensions/content/update.js	1.11
mozilla/toolkit/mozapps/extensions/content/update.xml	1.3
mozilla/toolkit/mozapps/extensions/src/nsExtensionManager.js.in 	1.117