Closed Bug 474289 Opened 15 years ago Closed 13 years ago

Automatically install add-ons shipped with the application into new profiles

Categories

(Toolkit :: Add-ons Manager, enhancement, P2)

enhancement

Tracking

()

VERIFIED FIXED
mozilla2.0b10
Tracking Status
status2.0 --- wanted

People

(Reporter: jwkbugzilla, Assigned: mossop)

References

Details

(Keywords: dev-doc-complete, Whiteboard: [rewrite])

Attachments

(1 file, 4 obsolete files)

TomTom HOME has an extension that is distributed both pre-installed with the application and as a downloadable XPI (for the case that an update is necessary before an application update). The downloadable version of the extension is always installed in user's profile which by itself is ok. We get a problem however when application is updated - suddenly the version in app directory is newer than the version in profile, yet the version in profile still takes precedence.

I think that install location priorities should be only a secondary mechanism for deciding which extension should be used. First step should be comparing the version number - if one of the extension instances is newer it should be used regardless of where it is installed. Only if the version number of several extension instances is identical priorities should be considered.

My impression from looking at the code is that this approach will break some optimizations - I hope that it can be done nevertheless.
As it stands this is intentional. The user install location has the highest priority even if the user has an older version there. See also bug 413153. However I am in the process of trying to come up with a better plan for managing the priorities of the install locations to try to fit what system admins and app developers might find more useful.
I guess implementing my suggestion above is a waste of time then. Dave, any idea in which direction your "better plan" will be going?

Sounds like implementing something similar to what is suggested in bug 413153 is our best option right now.
(In reply to comment #2)
> I guess implementing my suggestion above is a waste of time then. Dave, any
> idea in which direction your "better plan" will be going?

It's a little early yet, there is a thread on the newsgroup where I proposed some changes to the install locations and ended up realising that there were lots of different scenarios and we didn't really support many of them well, so the first step is to come up with a list of those scenarios and decide what their relative importance is. This is what I'm doing when I find spare minutes.

http://groups.google.com/group/mozilla.dev.platform/browse_thread/thread/a9ca75b3747c9c4e/22e4b7d3c7e9ab66
From that discussion Benjamin's idea sounds most interesting. The complete solution would be something like this: leave appdir/extensions to appManaged extensions (maybe make appManaged implicit there) and introduce another directory for "default extensions", maybe appdir/defaults/extensions. On application upgrade a component should then go through all extensions in appdir/defaults/extensions and install them in profile if either they aren't installed there or an older version is installed. Complicating factor: it should respect user's choice. This can be done by remembering which extensions/versions have been previously installed automatically. So if we installed the extension before and it isn't there any more this means that the user uninstalled it - don't attempt installation any more. If we installed the extension before but the version currently in profile is older - user explicitly chose to downgrade, don't upgrade him automatically.

Writing code that will do that for us shouldn't be a big problem but ideally extension manager should handle things like this itself.
(In reply to comment #4)
> From that discussion Benjamin's idea sounds most interesting. The complete
> solution would be something like this: leave appdir/extensions to appManaged
> extensions (maybe make appManaged implicit there) and introduce another
> directory for "default extensions", maybe appdir/defaults/extensions. On
> application upgrade a component should then go through all extensions in
> appdir/defaults/extensions and install them in profile if either they aren't
> installed there or an older version is installed. Complicating factor: it
> should respect user's choice. This can be done by remembering which
> extensions/versions have been previously installed automatically. So if we
> installed the extension before and it isn't there any more this means that the
> user uninstalled it - don't attempt installation any more. If we installed the
> extension before but the version currently in profile is older - user
> explicitly chose to downgrade, don't upgrade him automatically.
> 
> Writing code that will do that for us shouldn't be a big problem but ideally
> extension manager should handle things like this itself.

This does sound like something we should have in some form. Benjamin and I agree however that it should just be the simpler case. On first run of a new version of an application any bundled default extensions that hadn't already been installed previously would be installed to the profile. So there wouldn't be any upgrading of extensions in the profile from the application defaults. Instead all upgrades should be delivered using the usual add-on upgrade path.

The suggestion was to have the defaults appear in appdir/distribution alongside the other distribution stuff and use a pref distribution.extension.<ID>.installed to flag whether an add-on has been installed in this manner previously.

There are a few places where the code could go but it probably does make most sense to include it in the extension manager since that will be turned on/off depending on whether the application has extensions enabled. The checkForMismatches method ought to be a good starting point.
Severity: normal → enhancement
Priority: -- → P2
Summary: Old extension version in profile takes precedence over new version in app directory → Automatically install add-ons shipped with the application into new profiles
Whiteboard: [rewrite]
status2.0: --- → wanted
Attached patch patch rev 1 (obsolete) — Splinter Review
Mostly complete, needs one more in-depth test that extensions with more than one file actually work right.

This implements basically that described above, when a new version of an app runs any distributed extensions are installed unless a newer version exists in the profile or the extension had previously been installed by a distribution and since uninstalled. This allows upgrades at the moment unlike what I said above. In retrospect I think this is probably a useful additional feature.
Assignee: nobody → dtownsend
Status: NEW → ASSIGNED
Attached patch directory service patch (obsolete) — Splinter Review
This patch adds XREAppDist to the directory service pointing to the distribution directory. The only reason this is necessary is to be able to override it in the tests.
Attachment #503619 - Attachment is obsolete: true
Attachment #503982 - Flags: review?(benjamin)
Attached patch main patch rev 1 (obsolete) — Splinter Review
This is the main part of the patch. It implements the behaviour above and tests it. The only known issue that I think we can live with for now is that it doesn't respect em:unpack or the global preference when installing the add-on, instead if the add-on exists as an xpi in the distribution dir then that is how it is installed and similarly for if it exists as a directory.
Attachment #504001 - Flags: review?(robert.bugzilla)
Blocks: 594858
Whiteboard: [rewrite] → [rewrite][has patch]
What about extensions in XRE_SYS_LOCAL_EXTENSION_PARENT_DIR and XRE_SYS_SHARE_EXTENSION_PARENT_DIR ?
Comment on attachment 504001 [details] [diff] [review]
main patch rev 1

>diff --git a/toolkit/mozapps/extensions/XPIProvider.jsm b/toolkit/mozapps/extensions/XPIProvider.jsm
>--- a/toolkit/mozapps/extensions/XPIProvider.jsm
>+++ b/toolkit/mozapps/extensions/XPIProvider.jsm
>@@ -69,16 +69,17 @@ const PREF_EM_ENABLED_SCOPES          = 
> const PREF_EM_SHOW_MISMATCH_UI        = "extensions.showMismatchUI";
> const PREF_EM_DISABLED_ADDONS_LIST    = "extensions.disabledAddons";
> const PREF_XPI_ENABLED                = "xpinstall.enabled";
> const PREF_XPI_WHITELIST_REQUIRED     = "xpinstall.whitelist.required";
> const PREF_XPI_WHITELIST_PERMISSIONS  = "xpinstall.whitelist.add";
> const PREF_XPI_BLACKLIST_PERMISSIONS  = "xpinstall.blacklist.add";
> const PREF_XPI_UNPACK                 = "extensions.alwaysUnpack";
> const PREF_INSTALL_REQUIREBUILTINCERTS = "extensions.install.requireBuiltInCerts";
>+const PREF_INSTALLED_ADDON            = "extensions.installedAddon.";
nit: PREF_BRANCH_INSTALLED_ADDON

>@@ -189,17 +191,30 @@ SafeMoveOperation.prototype = {
>     catch (e) {
>       ERROR("Failed to move file " + aFile.path + " to " +
>             aTargetDirectory.path, e);
>       throw e;
>     }
>     this._movedFiles.push({ oldFile: oldFile, newFile: newFile });
>   },
> 
>-  _moveDirectory: function(aDirectory, aTargetDirectory) {
>+  _copyFile: function(aFile, aTargetDirectory) {
nit: *might* be a tad cleaner to just have an _installFile function with an aCopy param and combines _copyFile and _moveFile.

>+    let newFile = aFile.clone();
>+    try {
>+      newFile.copyTo(aTargetDirectory, null);
>+    }
>+    catch (e) {
>+      ERROR("Failed to copy file " + aFile.path + " to " +
>+            aTargetDirectory.path, e);
>+      throw e;
>+    }
>+    this._movedFiles.push({ oldFile: null, newFile: newFile });
>+  },
>+
>+  _moveDirectory: function(aDirectory, aTargetDirectory, aCopy) {
nit: since this can be a copy or move it would be a good thing to rename it to something like _installDirectory

>     let newDir = aTargetDirectory.clone();
>     newDir.append(aDirectory.leafName);
>     try {
>       newDir.create(Ci.nsILocalFile.DIRECTORY_TYPE, FileUtils.PERMS_DIRECTORY);
>     }
>     catch (e) {
>       ERROR("Failed to create directory " + newDir.path, e);
>       throw e;
>@@ -215,66 +230,94 @@ SafeMoveOperation.prototype = {
>...
>-  _moveDirEntry: function(aDirEntry, aTargetDirectory) {
>+  _moveDirEntry: function(aDirEntry, aTargetDirectory, aCopy) {
nit: since this can be a copy or move it would be a good thing to rename it to something like _installDirEntry

>     try {
>       if (aDirEntry.isDirectory())
>-        this._moveDirectory(aDirEntry, aTargetDirectory);
>+        this._moveDirectory(aDirEntry, aTargetDirectory, aCopy);
>+      else if (aCopy)
>+        this._copyFile(aDirEntry, aTargetDirectory);
>       else
>         this._moveFile(aDirEntry, aTargetDirectory);
>     }
>     catch (e) {
>-      ERROR("Failure moving " + aDirEntry.path + " to " + aTargetDirectory.path);
>+      ERROR("Failure " + (aCopy ? "copying" : "moving") + " " + aDirEntry.path +
>+            " to " + aTargetDirectory.path);
>       throw e;
>     }
>   },
> 
>   /**
>    * Moves a file or directory into a new directory. If an error occurs then all
>    * files that have been moved will be moved back to their original location.
>    *
>    * @param  aFile
>    *         The file or directory to be moved.
>    * @param  aTargetDirectory
>    *         The directory to move into, this is expected to be an empty
>    *         directory.
>    */
>   move: function(aFile, aTargetDirectory) {
>     try {
>-      this._moveDirEntry(aFile, aTargetDirectory);
>+      this._moveDirEntry(aFile, aTargetDirectory, false);
>+    }
>+    catch (e) {
>+      this.rollback();
>+      throw e;
>+    }
>+  },
>+
>+  /**
>+   * Copies a file or directory into a new directory. If an error occurs then
>+   * all files that have been copied will be removed.
>+   *
>+   * @param  aFile
>+   *         The file or directory to be copied.
>+   * @param  aTargetDirectory
>+   *         The directory to copy into, this is expected to be an empty
>+   *         directory.
>+   */
>+  copy: function(aFile, aTargetDirectory) {
nit: might be cleaner to just have a single install function that handles move and copy and has an aCopy param.

>+    try {
>+      this._moveDirEntry(aFile, aTargetDirectory, true);
>     }
>     catch (e) {
>       this.rollback();
>       throw e;
>     }
>   },
> 
>   /**
cc'ing Kev to give him a heads up since this *might* make Firefox 4.0
I think we are going to need something like appManaged added back so it is possible to differentiate between add-ons that will be updated by an app update and add-ons that won't so it is possible for app update to not include these add-ons in the incompatible list.

Thoughts?
Comment on attachment 504001 [details] [diff] [review]
main patch rev 1

>diff --git a/toolkit/mozapps/extensions/XPIProvider.jsm b/toolkit/mozapps/extensions/XPIProvider.jsm
>--- a/toolkit/mozapps/extensions/XPIProvider.jsm
>+++ b/toolkit/mozapps/extensions/XPIProvider.jsm
>@@ -1797,16 +1851,134 @@ var XPIProvider = {
>         // Non-critical, just saves some perf on startup if we clean this up.
>         LOG("Error removing staging dir " + stagingDir.path, e);
>       }
>     });
>     return changed;
>   },
> 
>   /**
>+   * Installs any add-ons bundled with the application into the profile unless
>+   * a newer version already exists or the user has previously uninstalled the
>+   * bundled add-on.
Needs @param aManifests

>+   *
>+   * @return true if any new add-ons were installed
>+   */
>+  installBundledAddons: function XPI_installBundledAddons(aManifests) {
>...
>+      if (existingEntry) {
>+        let existingAddon;
>+        try {
>+          existingAddon = loadManifestFromFile(existingEntry);
>+
>+          if (Services.vc.compare(addon.version, existingAddon.version) <= 0)
>+            continue;
>+        }
>+        catch (e) {
>+          // Bad add-on in the profile so just proceed and install over the top
I think WARN should be called here

>+        }
>+      }
>+      else if (Prefs.getBoolPref(PREF_INSTALLED_ADDON + id, false)) {
>+        continue;
>+      }
>+
>+      // Install the add-on
>+      try {
>+        profileLocation.installAddon(id, entry, null, true);
>+        LOG("Installed bundled add-on " + id);
>+
>+        Services.prefs.setBoolPref(PREF_INSTALLED_ADDON + id, true)
>+
>+        // Keep the add-on's manifest ready for adding to the database later
>+        // also removes any old manifest that might be there from a pending
>+        // install
This comment is a tad unclear

>@@ -6846,19 +7016,22 @@ DirectoryInstallLocation.prototype = {
>    * Installs an add-on into the install location.
>    *
>    * @param  aId
>    *         The ID of the add-on to install
>    * @param  aSource
>    *         The source nsIFile to install from
>    * @param  aExistingAddonID
>    *         The ID of an existing add-on to uninstall at the same time
>+   * @param  aCopy
>+   *         If false the source files will be moved to the new location,
>+   *         otherwise they will only be copied
>    * @return an nsIFile indicating where the add-on was installed to
>    */
>-  installAddon: function DirInstallLocation_installAddon(aId, aSource, aExistingAddonID) {
>+  installAddon: function DirInstallLocation_installAddon(aId, aSource, aExistingAddonID, aCopy) {
nit: this line is getting a tad long.

>     let trashDir = this.getTrashDir();
> 
>     let transaction = new SafeMoveOperation();
> 
>     let self = this;
>     function moveOldAddon(aId) {
>       let file = self._directory.clone().QueryInterface(Ci.nsILocalFile);
>       file.append(aId);
>@@ -6876,20 +7049,25 @@ DirectoryInstallLocation.prototype = {
> 
>     // If any of these operations fails the finally block will clean up the
>     // temporary directory
>     try {
>       moveOldAddon(aId);
>       if (aExistingAddonID && aExistingAddonID != aId)
>         moveOldAddon(aExistingAddonID);
> 
>-      if (aSource.isFile())
>-        Services.obs.notifyObservers(aSource, "flush-cache-entry", null);
>-
>-      transaction.move(aSource, this._directory);
>+      if (aCopy) {
>+        transaction.copy(aSource, this._directory);
>+      }
>+      else {
>+        if (aSource.isFile())
>+          Services.obs.notifyObservers(aSource, "flush-cache-entry", null);
Would an update to an existing install need a flush-cache-entry for the aCopy case?

>+
>+        transaction.move(aSource, this._directory);
>+      }
>     }
>     finally {
>       // It isn't ideal if this cleanup fails but it isn't worth rolling back
>       // the install because of it.
>       try {
>         recursiveRemove(trashDir);
>       }
>       catch (e) {
Comment on attachment 504001 [details] [diff] [review]
main patch rev 1

Definitely would like to look through this again and get answers to the questions before +'ing. I'm also a tad concerned that applications will start using this and there is no way to disable this for tests.
Attachment #504001 - Flags: review?(robert.bugzilla) → review-
Attachment #503982 - Flags: review?(benjamin) → review+
(In reply to comment #9)
> What about extensions in XRE_SYS_LOCAL_EXTENSION_PARENT_DIR and
> XRE_SYS_SHARE_EXTENSION_PARENT_DIR ?

This is entirely separate to the install locations for now. In this case we can get away with only checking on the startup of a new app, to do similarly elsewhere would mean a startup hit all the time.
(In reply to comment #13)
> > 
> >     // If any of these operations fails the finally block will clean up the
> >     // temporary directory
> >     try {
> >       moveOldAddon(aId);
> >       if (aExistingAddonID && aExistingAddonID != aId)
> >         moveOldAddon(aExistingAddonID);
> > 
> >-      if (aSource.isFile())
> >-        Services.obs.notifyObservers(aSource, "flush-cache-entry", null);
> >-
> >-      transaction.move(aSource, this._directory);
> >+      if (aCopy) {
> >+        transaction.copy(aSource, this._directory);
> >+      }
> >+      else {
> >+        if (aSource.isFile())
> >+          Services.obs.notifyObservers(aSource, "flush-cache-entry", null);
> Would an update to an existing install need a flush-cache-entry for the aCopy
> case?

That is taken care of above in moveOldAddon.
(In reply to comment #12)
> I think we are going to need something like appManaged added back so it is
> possible to differentiate between add-ons that will be updated by an app update
> and add-ons that won't so it is possible for app update to not include these
> add-ons in the incompatible list.
> 
> Thoughts?

We might want to think about that but it is possible for apps to just publish the version of the add-on that ships with the next version of the app in the extension's update.rdf and that should stop app update from warning about it right?

I'd rather avoid making more changes here for the time being and see how it works out.
Attached patch main patch rev 2 (obsolete) — Splinter Review
Updated patch address comments, adds a pref to disable installing add-ons (it defaults to on so that distributors need to make the least amount of changes to the app) and also including some test files that got missed last time.
Attachment #504001 - Attachment is obsolete: true
Attachment #504857 - Flags: review?(robert.bugzilla)
Comment on attachment 504857 [details] [diff] [review]
main patch rev 2

>diff --git a/toolkit/mozapps/extensions/XPIProvider.jsm b/toolkit/mozapps/extensions/XPIProvider.jsm
>--- a/toolkit/mozapps/extensions/XPIProvider.jsm
>+++ b/toolkit/mozapps/extensions/XPIProvider.jsm
>@@ -69,16 +69,18 @@ const PREF_EM_ENABLED_SCOPES          = 
> const PREF_EM_SHOW_MISMATCH_UI        = "extensions.showMismatchUI";
> const PREF_EM_DISABLED_ADDONS_LIST    = "extensions.disabledAddons";
> const PREF_XPI_ENABLED                = "xpinstall.enabled";
> const PREF_XPI_WHITELIST_REQUIRED     = "xpinstall.whitelist.required";
> const PREF_XPI_WHITELIST_PERMISSIONS  = "xpinstall.whitelist.add";
> const PREF_XPI_BLACKLIST_PERMISSIONS  = "xpinstall.blacklist.add";
> const PREF_XPI_UNPACK                 = "extensions.alwaysUnpack";
> const PREF_INSTALL_REQUIREBUILTINCERTS = "extensions.install.requireBuiltInCerts";
>+const PREF_INSTALL_BUNDLED_ADDONS     = "extensions.installBundledAddons";
The distribution dir already has a bundles concept and directory. Perhaps, PREF_INSTALL_DISTRO_ADDONS or PREF_INSTALL_DISTRIBUTION_ADDONS and extensions.installDistributionAddons?
http://mxr.mozilla.org/mozilla-central/source/toolkit/xre/nsXREDirProvider.cpp#572

>+const PREF_BRANCH_INSTALLED_ADDON     = "extensions.installedAddon.";
Perhaps extensions.installedDistroAddon. or similar

> 
> const URI_EXTENSION_UPDATE_DIALOG     = "chrome://mozapps/content/extensions/update.xul";
> 
> const DIR_EXTENSIONS                  = "extensions";
> const DIR_STAGE                       = "staged";
> const DIR_XPI_STAGE                   = "staged-xpis";
> const DIR_TRASH                       = "trash";
> 

>@@ -166,40 +169,43 @@ var gIDTest = /^(\{[0-9a-f]{8}-[0-9a-f]{
>  * A safe way to move a file or the contents of a directory to a new directory.
>  * The move is performed recursively and if anything fails an attempt is made to
>  * rollback the entire operation. The operation may also be rolled back to its
>  * original state after it has completed by calling the rollback method.
>  *
>  * Moves can be chained. Calling move multiple times will remember the whole set
>  * and if one fails all of the move operations will be rolled back.
nit: update the comment now that it also does copy

>  */
>-function SafeMoveOperation() {
>+function SafeInstallOperation() {
>   this._movedFiles = [];
nit: _installedFiles

>   this._createdDirs = [];
> }

>@@ -1797,16 +1840,139 @@ var XPIProvider = {
>         // Non-critical, just saves some perf on startup if we clean this up.
>         LOG("Error removing staging dir " + stagingDir.path, e);
>       }
>     });
>     return changed;
>   },
> 
>   /**
>+   * Installs any add-ons bundled with the application into the profile unless
"Installs add-ons located in the distribution directory's extensions directory..." or something similar

>+   * a newer version already exists or the user has previously uninstalled the
>+   * bundled add-on.
>+   *
>+   * @param  aManifests
>+   *         A dictionary to add new install manifests to to save having to
>+   *         reload them later
>+   * @return true if any new add-ons were installed
>+   */
>+  installBundledAddons: function XPI_installBundledAddons(aManifests) {
Same regarding distribution already having a bundle concept, etc.
Comment on attachment 504857 [details] [diff] [review]
main patch rev 2

Just nits so r+ing with the pref to disable installing add-ons from distribution/extensions added to automation.py so if an app starts using this it doesn't bust existing tests. If at some point someone wants this turned on for tests other than just changing the pref for xpcshell tests they can first verify that all tests pass before enabling it.

Also, I think the UX team should be given a heads up since this will add another - better way IMO - for add-ons to be installed without user notification.
Attachment #504857 - Flags: review?(robert.bugzilla) → review+
btw: if you want me to take another look at the updated patch don't hesitate to request review
Updated from comments and merged, ready to land after approval
Attachment #503982 - Attachment is obsolete: true
Attachment #504857 - Attachment is obsolete: true
Attachment #505184 - Flags: review+
Attachment #505184 - Flags: approval2.0?
This is a fairly well contained change in pretty well tested code and will give us the benefit of giving users full control over the Feedback add-on we ship with the next beta
Comment on attachment 505184 [details] [diff] [review]
full patch
[Checked in: Comment 25]

a=beltzner
Attachment #505184 - Flags: approval2.0? → approval2.0+
Landed: http://hg.mozilla.org/mozilla-central/rev/9d41d0fa20ad
Status: ASSIGNED → RESOLVED
Closed: 13 years ago
Flags: in-testsuite+
Flags: in-litmus-
Resolution: --- → FIXED
Whiteboard: [rewrite][has patch] → [rewrite]
Target Milestone: --- → mozilla2.0b10
Blocks: 627240
Keywords: dev-doc-needed
diff --git a/toolkit/mozapps/extensions/test/xpcshell/test_distribution.js b/toolkit/mozapps/extensions/test/xpcshell/test_distribution.js
--- a/toolkit/mozapps/extensions/test/xpcshell/test_distribution.js
+++ b/toolkit/mozapps/extensions/test/xpcshell/test_distribution.js
@@ -63,17 +63,17 @@ function setOldModificationTime() {
   // changes will be detected
   shutdownManager()
   let extension = gProfD.clone();
   extension.append("extensions");
   if (Services.prefs.getBoolPref("extensions.alwaysUnpack"))
     extension.append("addon1@tests.mozilla.org");
   else
     extension.append("addon1@tests.mozilla.org.xpi");
-  setExtensionModifiedTime(extension, Date.now - 10000);
+  setExtensionModifiedTime(extension, Date.now() - 10000);
   startupManager(false);
 }
 
 function run_test() {
   do_test_pending();
 
   run_test_1();
 }
Tested all possible paths, including upgrades from 3.6.x. Marking as verified fixed with Mozilla/5.0 (Macintosh; Intel Mac OS X 10.6; rv:2.0b10pre) Gecko/20110121 Firefox/4.0b10pre
Status: RESOLVED → VERIFIED
Version: unspecified → Trunk
Attachment #505184 - Attachment description: full patch → full patch [Checked in: Comment 25]
Quick question on this - does installation with this method support xpi expansion set in the XPI's install.rdf? Noticing with a couple partner bundles that it doesn't appear to, and I wanted to know if the always unpack pref needed to be set.
Kev, you mean that extracted XPI's in a former version of Firefox, do not get the newest version of the add-on installed into the users profile? I have tested this path and it worked fine for me. Do you have some examples?
Sorry, not very clear, there.

I've included an .xpi in the /core/distribution/extensions directory of the win32 Fx4 installer. in that xpi, the install.rdf has <em:expand>true</em:expand> set. the xpi is installed to the profile directory, but does not appear to expand properly (the xpi is installed, but needed data directories aren't created). I can mail you a sample.
(In reply to comment #30)
> I've included an .xpi in the /core/distribution/extensions directory of the
> win32 Fx4 installer. in that xpi, the install.rdf has
> <em:expand>true</em:expand> set. the xpi is installed to the profile directory,

I assume you want to use "<em:unpack>true</em:unpack>"?
Yes, I do, and I had expansion on the brain when I typed that. I do use "<em:unpack>true</em:unpack>". Apologies for introducing more confusion.
So yes please send me a sample I can use for testing, including the steps. If it's valid we should file a new bug, or do it immediately if you want.
(In reply to comment #28)
> Quick question on this - does installation with this method support xpi
> expansion set in the XPI's install.rdf? Noticing with a couple partner bundles
> that it doesn't appear to, and I wanted to know if the always unpack pref
> needed to be set.

As mentioned in comment 8 it currently ignores em:unpack. If the extension needs to be unpacked then it should be unpacked when shipped.
What here in this bug affects documentation? It's not immediately clear to me what new API is introduced here, or if it's just changes to existing stuff (and, for that matter, I'm not sure if this might have already been documented).
(In reply to comment #35)
> What here in this bug affects documentation? It's not immediately clear to me
> what new API is introduced here, or if it's just changes to existing stuff
> (and, for that matter, I'm not sure if this might have already been
> documented).

Similar to the documented global locations for installing extensions (https://developer.mozilla.org/en/Installing_extensions) we now have a new location at <appdir>/distribution/extensions and any extensions here get installed into a user's profile when they first run with that version of the application assuming they have never uninstalled that extension in the past and they don't have a newer version already present in their profile.
Depends on: 641746
No longer depends on: 641746
You need to log in before you can comment on or make changes to this bug.