I'd like to support install locations on the filesystem for per-user and all-users that do not require the need to locate the firefox install or the user's profile directory. Under windows this is possible via the registry. We could either support equivalent registry-like systems on Linux and OSX, or we could support standard filesystem locations. I wouldn't mind doing both. For the per-user locations, it makes sense to use "[UAppData]/extensions", which translates to the following locations: Windows: %USERPROFILE%\Application Data\Mozilla\Firefox\extensions Linux: ~/.mozilla/firefox/extensions OSX: ~/Library/Application Supports/Firefox/extensions However, I'm not sure what the correct install locations would be for extensions available to all users. Maybe the following: Windows: %PROGRAMFILES%\Common Files\Mozilla\Firefox\extensions Linux: /usr/lib/mozilla/firefox/extensions OSX: /Library/Application Supports/Firefox/extensions But, I'm really not sure about these locations. Suggestions?

This patch adds support for the per-user install locations enumerated above.

Comment on attachment 198407 [details] [diff] [review] v1 patch [fixed-on-trunk, backed-out] Would like your feedback on this...

Comment on attachment 198407 [details] [diff] [review] v1 patch [fixed-on-trunk, backed-out] This sounds fine with me. The global one sounds a bit iffier to me (/opt/mozilla/firefox/extensions sounds like a more natural place, but we should consult with caillon/wolfir).

caillon: any suggestions? (anyone know how to contact wolfir to ask him the same question?)

v1 patch fixed-on-trunk

how about [appinstalldir]/extensions?

If this is about extensions (app-specific) why not just use what biesi describes? It's already there. In SUSE Linux 10.0 we've introduced /usr/lib/browser-extensions/firefox/ for firefox-specific addon extensions. (We already had /usr/lib/browser-plugins)

(In reply to comment #6) > how about [appinstalldir]/extensions? What do you mean by [appinstalldir]?

the directory where firefox-bin is in.

(In reply to comment #9) > the directory where firefox-bin is in. Oh, well that is already supported. The challenge that I'm to solve with this bug is allowing people to install global extensions without having to know where firefox is (or will be) installed.

We've been using the extension's id to guarantee uniqueness per install location. Since this uses the same install location of PRIORITY_APP_SYSTEM_USER as is used by the registry it is possible - though admittedly an edgecase - that the same install location will contain two extensions with the same id.

(In reply to comment #11) > We've been using the extension's id to guarantee uniqueness per install > location. Since this uses the same install location of PRIORITY_APP_SYSTEM_USER > as is used by the registry it is possible - though admittedly an edgecase - that > the same install location will contain two extensions with the same id. I'm not sure I understand. Each install location has a unique priority value.

I just noticed that the winreg location adds 10 to the priority in nsExtensionManager.js.in and I thought they were defined in the idl along with the rest. I need to sleep more.

We're between beta2 and rc1 so it seems pretty darned late for a new feature. Can you tell us more about why this is needed so late in the game? Thanks.

(In reply to comment #14) > We're between beta2 and rc1 so it seems pretty darned late for a new feature. > Can you tell us more about why this is needed so late in the game? Thanks. It's just "a nice to have." It gives third-party installers the ability to install extension (on all platforms) without having to figure out where Firefox is installed. It also allows extensions to be installed before Firefox is installed. We currently only support these features on Windows via special registry keys. So, this bug is about adding feature parity to other platforms. I can understand wanting to avoid risk at this point in the game. This is definitely a low risk patch, and the upside is pretty good in my opinion. There's not much that can go wrong with this patch.

Comment on attachment 198407 [details] [diff] [review] v1 patch [fixed-on-trunk, backed-out] let's get it for the next release. We're too late for nice to haves.

per the triage mtg and the bug approval flag.

I know I marked r+ here, but I think I'd like this to use appid instead of appname to register extensions.

I'm a bit puzzled. So trunk with this patch on Linux/Unix searches for extensions in - [where firefox-bin is]/extensions - $HOME/.mozilla/firefox/XYZPROFILE/extensions *and new* - $HOME/.mozilla/firefox/extensions ?

As outlined above we use /usr/lib/browser-extensions/firefox at the moment to install global extensions. This is the patch we use to include it. I would prefer to have a default path somewhere hardcoded which can be overwritten with environment variables. But this does it for now (Firefox 1.5).

Comment on attachment 198407 [details] [diff] [review] v1 patch [fixed-on-trunk, backed-out] We should get this on the MOZILLA_1_8_BRANCH

so where exactly would this search for mac? i just want to make sure it's system-wide, not user-specific.

The current patch adds a new location that is normally located on Mac OS X as follows: ~/Library/Application Support/Firefox/Profiles/extensions/

(In reply to comment #22) > so where exactly would this search for mac? i just want to make sure it's > system-wide, not user-specific. to clarify, there is one new location that is user specific and the additional locations for system wide have as yet to be defined.

Rob, I thought we decided (after I reviewed this patch) that we really wanted to do this with app IDs instead of names...

so for linux it's system wide, but for mac it's per-user? that seems odd. am i missing something?

(In reply to comment #25) > Rob, I thought we decided (after I reviewed this patch) that we really wanted > to do this with app IDs instead of names... For the user wide location (e.g. multiple app profiles using the same install location like Mozilla/Firefox/profiles/extensions/)? Not that I know of but if that is the case this should be backed out on the trunk. (In reply to comment #26) > so for linux it's system wide, but for mac it's per-user? that seems odd. am i > missing something? No, for Linux it is ~/.mozilla/firefox/extensions/ so it is an install location for all profiles for a single user on a system.

Comment on attachment 198407 [details] [diff] [review] v1 patch [fixed-on-trunk, backed-out] Clearing 1.8.1 flag - per bsmedberg we need this to use app ID's for cross app integration with XULRunner. With that in mind this patch should probably be backed out on the trunk.

Comment on attachment 198407 [details] [diff] [review] v1 patch [fixed-on-trunk, backed-out] Lets get this into FF2 :)

Comment on attachment 198407 [details] [diff] [review] v1 patch [fixed-on-trunk, backed-out] Darin, see discussion: I'd really like this to map IDs instead of vendor/names.

Ugh... app IDs (GUIDs) are not very user-friendly. For the Win32 registry based install location we use app name, not app ID. That said, I fully understand why app name is insufficient for cross-app extension installation. I backed the v1 patch out of the trunk.

bsmedberg: Do you have a proposal for the install locations? What directories should contain the app ID directories?

-> default owner

Something off-topic: In Windows, Firefox user profile is in %USERPROFILE%\Application Data\Mozilla\Firefox\ but Thunderbird's is in %USERPROFILE%\Application Data\Thunderbird\ I would consider it as a bug. Is it filed?

~/.mozilla-extensions/<appID> ? The "mozilla" is a little weird... got a better suggestion?

.xulrunner-extensions?

Whew. Okay, this patch combines both ideas from Darin and Wolfgang, and uses appID instead of the name as suggested by bsmedberg. This patch also supports multilib, which would be a requirement for some distros, such as Fedora, RHEL, and CentOS. On these, we want to look in /usr/lib64/xulrunner/extensions/<appID> if we're using the 64bit XR and /usr/lib/xulrunner/extensions/<appID> if we're using the 32bit version. The problem here is that not all distros do multilib. These want /usr/lib/xulrunner/extensions/<appID>. Since I can't look at the arch or anything like sizeof(void*) to determine whether we care about multilib or not, I think the best way to do this is to see whether our LIBDIR is /usr/lib64 which it would be if we're building the 64bit version and need to look for 64bit compiled extensions on a multilib setup. /usr/share is the place where non-compiled extensions go, so we can avoid installing the exact same copy on disk. This is compatible with the XDG datadir location. Finally, this also supports $HOME/.xulrunner/extensions/<appID> which is what darin tried to patch. This patch actually should support all platforms, by moving some code around. I think I got this bit right, judging by what darin's patch did. One final note is that I didn't implement the global system dir stuff for mac or windows because well, I'm not really that knowledgable about those platforms nor do I have access to those OSes. But I hope that doesn't slow down getting this patch in because this would make using XR+FF sooo much nicer for us on *nix. Comments?

Taken a quick skim over this and it generally looks great. The only thing I'd question is the use of "xulrunner" in the directory paths. The patch as it stands affects all toolkit based apps (correctly I believe), not just those running on a XULRunner runtime so using the xulrunner name feels a little weird to me. Perhaps we should use "xre" instead? For the global system dir stuff, for Mac I think we should use "/Library/Application Support" to match the user specific directory. I don't think we necessarily need one for Windows given that we have registry support already (we may not even need the user specific one for the same reason) but if we did it should probably be under "C:\Documents and Settings\All Users\Application Data". Both of those could be done in a further patch though. One irritation is that the windows registry locations use vendor and app name instead of the app id. Might be an idea to add further registry locations using just the ID for consistency, not sure what kind of startup cost all these additional locations is going to incur though.

Regarding Windows support, I'd rather see less registry support, not more, so I'd like to see a global & user system dirs for Windows. The registry can be confusing to setup correctly, can have permission issues, isn't as easy to programmatically tweak as files and consistency between platforms would be nice. But, I agree these could be done in a future patch.

(In reply to comment #38) > Taken a quick skim over this and it generally looks great. The only thing I'd > question is the use of "xulrunner" in the directory paths. The only part I'm really tied to is: /usr/{lib,lib64,share} so I'll be more than happy to change the "xulrunner" part to whatever. I guess the "xulrunner" name doesn't make too much sense for non-XR apps, but it's probably no worse than the global plugins directory being /usr/lib{,64}/mozilla/plugins. Hmmmmmm, so come to think about it, maybe we should go with /usr/{lib,lib64,share}/mozilla/extensions? Again, no real preferences for it. I'll let the reviewers decide. :-)

Comment on attachment 295576 [details] [diff] [review] New version I've asked Mossop to review the nsExtensionManager.js parts, because that's not my specialty. Here are my nits on the rest: >Index: toolkit/xre/Makefile.in You don't need to do libdir = @libdir@ ... it's already substituted in autoconf.mk >+ifeq ($(OS_ARCH),Linux) >+ifeq ($(libdir),/usr/lib64) >+DEFINES += -DHAVE_USR_LIB64_DIR >+endif >+endif Why not just use libdir directly? And while you're at it, datadir? Is this because you want Firefox compiled --prefix=/usr/local to still load extensions from /usr/lib* ? DEFINES += -DNS_libdir=\"$(libdir)\" -DNS_datadir=\"$(datadir)\" If this isn't ok, I wonder if we should make this configurable and use an autoconf substitution, rather than coding the logic in a makefile... >Index: toolkit/xre/nsXREDirProvider.cpp >+#ifdef XP_UNIX >+ else if (!strcmp(aProperty, XRE_SYS_LOCAL_EXTENSION_PARENT_DIR)) { >+ >+ static const char *const sysSExtDir = >+#ifdef HAVE_USR_LIB64_DIR >+ "/usr/lib64/xulrunner/extensions" >+#else >+ "/usr/lib/xulrunner/extensions" >+#endif static const char[] sysSExtDir = NS_libdir "/xulrunner/extensions";

Oh, and I like the /usr/lib/mozilla/extensions suggestion... let's do that.

(In reply to comment #41) > >+ifeq ($(OS_ARCH),Linux) > >+ifeq ($(libdir),/usr/lib64) > >+DEFINES += -DHAVE_USR_LIB64_DIR > >+endif > >+endif > > Why not just use libdir directly? And while you're at it, datadir? Is this > because you want Firefox compiled --prefix=/usr/local to still load extensions > from /usr/lib* ? Yes, that was my reasoning. The system extension dir should work with more than just the system supplied builds, IMO...

as a sidenote: additionally, i would like libxul applications also to consider $GRE_DIR/{extensions,plugins}

I do not want GRE_DIR/{extensions,plugins}... XR shouldn't be shipping with plugins/extensions. What's the use-case you're worried about?

Comment on attachment 295576 [details] [diff] [review] New version There's a few changes to be made here but this is mostly good: > // Force an update of the metadata for appManaged extensions since the > // last modified time is not updated for directories on FAT / FAT32 > // filesystems when software update applies a new version of the app. > var location = this.getInstallLocation(id); >- if (location.name == KEY_APP_GLOBAL) { >+ if (location.name == KEY_APP_GLOBAL || >+ location.name == KEY_APP_SYSTEM_LOCAL || location.name == KEY_APP_SYSTEM_SHARE) { > var installRDF = location.getItemFile(id, FILE_INSTALL_MANIFEST); > if (installRDF.exists()) { This change is unnecessary, it is to force-update metadata from add-ons that are installed with the application, and is really only necessary on FAT filesystems. If we have problems detecting updates to add-ons in these new locations then we would have to do something elsewhere on every startup. > // Prepare themes for installation >- // Only enumerate directories in the app-profile and app-global locations. >- var locations = [KEY_APP_PROFILE, KEY_APP_GLOBAL]; >+ // Only enumerate directories in the app-profile, app-global, app-system-share and app-system-user locations. >+ var locations = [KEY_APP_PROFILE, KEY_APP_GLOBAL, KEY_APP_SYSTEM_SHARE, KEY_APP_SYSTEM_USER]; > for (var i = 0; i < locations.length; ++i) { > var location = InstallLocations.get(locations[i]); >- if (!location.canAccess) >+ if (!location || !location.canAccess) > continue; Probably shouldn't include the SYSTEM_SHARE area since if we find anything we have to make changes which we talked about being a bad idea. I'm not sure we even need the SYSTEM_USER, did any version of Firefox 1.0 have files there? > } > } > else if (installLocation.name == KEY_APP_PROFILE || >- installLocation.name == KEY_APP_GLOBAL) { >+ installLocation.name == KEY_APP_GLOBAL || >+ installLocation.name == KEY_APP_SYSTEM_LOCAL || >+ installLocation.name == KEY_APP_SYSTEM_SHARE || >+ installLocation.name == KEY_APP_SYSTEM_USER) { > // Check for a pointer file and remove it if it exists > var pointerFile = installLocation.location.clone(); > pointerFile.append(id); > if (pointerFile.exists() && !pointerFile.isDirectory()) > pointerFile.remove(false); > } Not sure we should even support pointer files for these locations but I guess there isn't much harm. However we shouldn't be trying to remove them from the SYSTEM_LOCAL and SYSTEM_SHARE locations. Same goes for the 2nd case like this. > _rdfGet_appManaged: function(item, property) { > var id = stripPrefix(item.Value, PREFIX_ITEM_URI); > var locationKey = this.getItemProperty(id, "installLocation"); >- if (locationKey != KEY_APP_GLOBAL) >+ if (locationKey != KEY_APP_GLOBAL && locationKey != KEY_APP_SYSTEM_LOCAL && locationKey != KEY_APP_SYSTEM_SHARE) > return EM_L("false"); > return null; This shouldn't be here. As it stands you haven't made the change to allow items in these locations to be appManaged anyway. And I don't think we should. appManaged is for extensions included in the application install package which I don't think includes the other system locations. In addition there needs to be a change to RDFItemUpdater.checkForUpdates to stop us checking for updates to the system install locations. What I would suggest is making a change to DirInstallLocation to allow you to make it return true for itemIsManagedIndependently for your system locations (just a new constructor parameter for independent locations). Then you can just replace the check for the winreg locations with that test. Should that be true for the system user location as well?

Thanks Mossop. This incorporates your feedback. Bsmedberg: I removed libdir = @libdir@ but I'm not sure how to best address the other nit yet and still keep the behavior of defaulting to /usr/{lib,lib64,share} when building without --prefix=/usr or whatnot. I think it's pretty important for us to just pick something and go with it which will make it easier for anyone that wants to ship 3rd party extension distro RPMs, etc. Can we go with this for now and see about improving it later?

Comment on attachment 296109 [details] [diff] [review] Updated per comments Excellent, r=me for the nsExtensionManager.js part.

Comment on attachment 296109 [details] [diff] [review] Updated per comments >Index: toolkit/xre/Makefile.in >+ifeq ($(OS_ARCH),Linux) >+ifeq ($(libdir),/usr/lib64) >+DEFINES += -DHAVE_USR_LIB64_DIR >+endif >+endif If you wanted to "catch" libdir == /usr/local/lib64, you could do ifneq (,$(findstring lib64,$(libdir))) >Index: toolkit/xre/nsXREDirProvider.cpp >+#ifdef XP_UNIX Please make this #if defined(XP_UNIX) && !defined(XP_MACOSX) r=me with that change

Comment on attachment 296109 [details] [diff] [review] Updated per comments Seeking approval. This greatly helps out distros, since we are able to ship extensions now without needing to guess where to install them. Since GNOME and other projects like Mugshot are starting to ship them, it is a huge burden without something like this.

Comment on attachment 296109 [details] [diff] [review] Updated per comments a=beltzner for 1.9

Discussed this patch with caillon on IRC, we came to the conclusion that hardcoding the system path by default makes sense because it will have the effect that all Linux versions work the same way (rather than forcing people to compile a Firefox extension once for SuSE, once for Fedora, once for Debian, etc.). However, we also realized it would be useful to have a developer override ability; like how one can say PYTHONPATH=/src/build/mypython-stuff. I'll file a new bug.

This has been checked in with bsmedberg's comments from comment 49 addressed and I needed an additional change to nsExtensionManager.js.in to also add a wrapper to make sure that XP_MACOSX wasn't defined there.

Sheppy, we need a devmo page analagous to http://developer.mozilla.org/en/docs/Adding_Extensions_using_the_Windows_Registry for this feature.

Filed bug 412449 for adding matching install locations on OSX and Windows.

See also bug 412446 for changes that would be useful for developers.

Hmm, interesting. The test bustage fix in nsExtensionManager.js.in caused trace-malloc-leaks to drop by ~65K on OS X. Nice!

(Bug 311008 filed on whatever was leaking)

You mean bug 412527

Oops. :)

You may want to check this post from the Trunk thread: http://forums.mozillazine.org/viewtopic.php?p=3220736#3220736 My experience has been that add-ons are not being installed on the first attempt. They appear to get to the point where their installation should be complete and then "things" abort (all add-on folder actually disappearing on my attempt today) and Minefield opens without the add-ons installed. Repeating the steps to retry and things work fine. Hhhmmm! <rubbing chin>

(In reply to comment #61) > You may want to check this post from the Trunk thread: > > http://forums.mozillazine.org/viewtopic.php?p=3220736#3220736 > > My experience has been that add-ons are not being installed on the first > attempt. They appear to get to the point where their installation should be > complete and then "things" abort (all add-on folder actually disappearing on my > attempt today) and Minefield opens without the add-ons installed. Repeating the > steps to retry and things work fine. Hhhmmm! <rubbing chin> Please file a bug for your issue, enabled extensions.logging.enabled and provide the errors that are occurring on startup. Also attach the extensions.log file from your profile folder.

First try using my existing profile yielded successful installation of add-ons and no log file generated. I'll try a new profile next. (In reply to comment #62) > (In reply to comment #61) > > You may want to check this post from the Trunk thread: > > > > http://forums.mozillazine.org/viewtopic.php?p=3220736#3220736 > > > > My experience has been that add-ons are not being installed on the first > > attempt. They appear to get to the point where their installation should be > > complete and then "things" abort (all add-on folder actually disappearing on my > > attempt today) and Minefield opens without the add-ons installed. Repeating the > > steps to retry and things work fine. Hhhmmm! <rubbing chin> > > Please file a bug for your issue, enabled extensions.logging.enabled and > provide the errors that are occurring on startup. Also attach the > extensions.log file from your profile folder. >

(In reply to comment #63) > First try using my existing profile yielded successful installation of add-ons > and no log file generated. I'll try a new profile next. Please file a new bug if you find any problems, your problem is unrelated to the checkin for this bug. This bug is closed now and does not need further comments.

Trying with a new profile, Minefield did hang on opening after installing add-ons, but the add-ons did install. (Used Task Manager to force Minefield closed.) Unlike when I tried installing earlier this morning, I did open Minefield without add-on files in the extensions folder because I had to enable extensions logging before installing add-on via this method. Could what I experienced be Minefield needing to be opened one time without add-ons installed in a profile or globally before this works? (In reply to comment #64) > (In reply to comment #63) > > First try using my existing profile yielded successful installation of add-ons > > and no log file generated. I'll try a new profile next. > > Please file a new bug if you find any problems, your problem is unrelated to > the checkin for this bug. This bug is closed now and does not need further > comments. >

What's the scenario in which this gets used? I'm a little confused. The article "Adding Extensions using the Windows Registry" looks like it's about writing installer applications to install extensions from outside Firefox, but this bug involves an update to nsExtensionManager.js, which would only be used inside Firefox (or other XUL based apps). Is that a correct assessment?

Sheppy, if an installer on Windows wants to install a Firefox extension, it should use the registry method, and Firefox will automatically look in the registry to find new extensions. Now we've added similar feature for Linux/Mac: If a third-party installer wants to install an extension (via RPM, for instance), they can unzip the extension to /usr/lib/mozilla/extensions/<extension ID> and all gecko-based apps will look in that directory and find compatible extensions. The Mac directory is /Library/Application Support/Mozilla/Extensions/<extension ID>

(In reply to comment #66) > What's the scenario in which this gets used? I'm a little confused. The > article "Adding Extensions using the Windows Registry" looks like it's about > writing installer applications to install extensions from outside Firefox, but > this bug involves an update to nsExtensionManager.js, which would only be used > inside Firefox (or other XUL based apps). > > Is that a correct assessment? The change to nsExtensionManager.js allows Firefox (and any toolkit application) to detect extensions installed at certain special directories, same as it currently detects extensions installed in the application directory and profile directory (and registry on windows). The special directories are: Linux: /usr/lib/mozilla/extensions/<appid> (or /usr/lib64/mozilla/extensions/<appid>) /usr/share/mozilla/extensions/<appid> ~/.mozilla/extensions/<appid> OSX: /Library/Application Support/Mozilla/Extensions/<appid> ~/Library/Application Support/Mozilla/Extensions/<appid> There are no changes for Windows.

OK, awesome, I totally misread this then. Fantastic, thank you.

Now documented: http://developer.mozilla.org/en/docs/Installing_extensions This article covers this technique for installing extensions on Mac and Linux, and provides a link to the Windows registry method article.

Why on earth require <appid> in the path ? It's making things painful for extensions that support several applications, which they also have the proper information in install.rdf !

Implemented a long time back and it works as expected. Marking as verified fixed.