The $MOZ_PLUGIN_PATH env var is currently expected to be just a single variable. It really should support the usual colon-delimited PATH syntax. I was reminded of this by the following comment in bug 13474: ------- Additional Comments From bzbarsky@mit.edu 2002-07-01 21:14 ------- Almost forgot. There's some really dumb PATH-walking code in uriloader/exthandler/unix/nsOSHelperAppService.cpp. It may make sense to prettify it some, make it a little more careful, and put it somewhere where this code can use it too...

assigning to serge

*** Bug 151208 has been marked as a duplicate of this bug. ***

This is actually for ALL platforms, Windows uses this variable too. With the current way we use the app directory service provider, this may be a little difficult to fix. Perhaps we can move the MOZ_PLUGIN_PATH key into it's own directory service provider?

It's not so hard. That env variable is used in a context where the provider is being asked for a list of files, it is not a publicly available key. http://lxr.mozilla.org/mozilla/source/xpcom/io/nsAppFileLocationProvider.cpp#509 In this code, if the environment var was parsed as a list of files, it could return multiple files. It would make the iterator object a little more complex though.

This is really bothering our (Ximian) users, its one of the most reported bugs after shipping Mozilla 1.0. Are there any patches floating around?

Conrad, you are that code owner, would you take this bug, please?

Taking it.

thanks.

ping?

So I'm guessing this won't be fixed in 1.2 either?

Since this is breaking plugins on EVERY mozilla release, I'd like to see it fixed for the next one. Adding nomination for mozilla1.2

frb tested a version of this on linux

Yes, this seems to work fine if the user/mozilla wrapper script sets MOZ_PLUGIN_PATH

Comment on attachment 103216 [details] [diff] [review] create a special class which parses the first path by the os path delim >+ static const char* keys[] = { nsnull, NS_USER_PLUGINS_DIR, NS_APP_PLUGINS_DIR, nsnull }; >+ if (!*keys) >+ keys[0] = PR_GetEnv("MOZ_PLUGIN_PATH"); Why the condition? You're guaranteeing that it's going to be true in the line above it, no?

static const char* keys[] = { nsnull, NS_USER_PLUGINS_DIR, NS_APP_PLUGINS_DIR, nsnull }; if (!*keys) keys[0] = PR_GetEnv("MOZ_PLUGIN_PATH"); > Why the condition? You're guaranteeing that it's going to be true in the line > above it, no? no :), but perhaps if you saw it like this it'd make more sense: if (!keys[0]) keys[0] = PR_GetEnv("MOZ_PLUGIN_PATH");

OK, so maybe I'm a moron, but I'm still not seeing it. You set char * keys to { nsnull, NS_USER_PLUGINS_DID, ... }. That means that keys[0] is nsnull. This in turn means that the condition (!*keys) or, equivalently, the condition (!keys[0]) will always evaluate to true, since keys[0] will always be nsnull/0/false. Did I lose my mind?

shaver, that's a static. So after the first call to that function it will not be null anymore.

Comment on attachment 103216 [details] [diff] [review] create a special class which parses the first path by the os path delim None of that happened. sr=shaver.

Comment on attachment 103216 [details] [diff] [review] create a special class which parses the first path by the os path delim >Index: mozilla/xpcom/io/nsAppFileLocationProvider.cpp > >+#if defined(XP_WIN) || defined(XP_OS2)/* Win32, Win16, and OS/2 */ >+#define PATH_SEPARATOR ';' >+#else /*if defined(XP_UNIX) || defined(XP_MAC) || defined(XP_BEOS)*/ Nice. Thanks for taking this. r=ccarlen One nit: remove the XP_MAC from the above comment. This doesn't apply to Mac and that definition, on 1st read, gave me a scare. >+#define PATH_SEPARATOR ':' >+#endif

Comment on attachment 103216 [details] [diff] [review] create a special class which parses the first path by the os path delim >+ static const char* keys[] = { nsnull, NS_USER_PLUGINS_DIR, NS_APP_PLUGINS_DIR, nsnull }; >+ if (!*keys) >+ keys[0] = PR_GetEnv("MOZ_PLUGIN_PATH"); to eliminate an expansive |PR_GetEnv| call I would change it to something like this: if (!keys[0] && !(keys[0] = PR_GetEnv("MOZ_PLUGIN_PATH"))) { static const char nullstr = 0; keys[0] = &nullstr; }

Comment on attachment 103216 [details] [diff] [review] create a special class which parses the first path by the os path delim a=dbaron for trunk checkin

checked in

verified that this works: chb@frodo:~/mozilla/opt/xpcom$ ls /tmp/p* /tmp/p1: nppdf.so /tmp/p2: libflashplayer.so $ MOZ_PLUGIN_PATH=/tmp/p1:/tmp/p2 ~/mozilla/opt/dist/bin/mozilla about:plugins then correctly showed both the PDF plugin and the Flash plugin. marking VERIFIED on linux using latest CVS (of the changed file only, other stuff is a bit older)