Just talked with peter on this one. I think we should also use the same logic on mac like on windows so that the shockwave plugin form 4.x is picked up and included in the plugin scan on mac. Since there are registration problems on mac( 35916) shockwave download, it causes a lot of problems to get the plugin actually working.

Changing summary

Oh wow, looking at the code, there was already an attempt to use a common folder called "Netscape Plugins" inside the Extensions folder. The code is there but turned off. I'm sure it would probably be easy to lookup the path of Netscape 4.x and scan it's plugins. What would be really cool is if we could pick up Mac Internet Explorer plugins too. From what I recall, they are compatible. Do we want to even do any of these? Perhaps all of this should be controled by a "Plugin Manager" but until then, pref flags would probably be the best way to control this. Can someone add some installer folks to the cc: list to see what they think. Also, perhaps they can also point me to some example code that allows one to find the installed path of a particular application by using the desktop database.

-->1.0

cc:ing Samir: Could you show me where in the tree I can find some example code for reading the installation path of Nav 4.x from Apple's Desktop Database? Thanks.

A quick glance at lxr for PBDTGetAPPL() revealed nothing relevant (installer and nsLocalFileMac only). You may want to consult the Desktop Manager docs on developer.apple.com. for PBDTGetAPPL() and friends. Simon, Is there sample code lying around to fulfill Peter's last request?

Peter, Maybe this will help you some although probably not reusable for your purposes: <http://lxr.mozilla.org/seamonkey/source/xpcom/io/nsLocalFileMac.cpp#233> You'll have to write something up that keeps querying the desktop until it finds an instance of the app with a 4.x version that exists (maybe *all* instances with a 4.x version that exists).

This would be ugly, and not something we'd want to do at each startup. Leave mucking with plugin installation to the installer, and plugin installers. Mozilla should not go grubbing for 4.x plugin folders. If you must do it, you have to do something like: for each mounted volume if that volumne has a desktop database get each app with signature 'MOSS' This is slow code that will cause lots of disk-rattling.

This is something I thought I shot down before 6.0 went out. Besides being slow, as sfraser mentions, there's also the issue of which version of an installed 4.x you'd want to use. Unlike Windows where you've pretty much got one path to one installed version of an app, the Mac lets you have N installed copies of the app. Not that I'm a typical user but I have 4 different versions of 4.x installed and I know folks outside the company that still keep Navigator 3 around. What would make more sense is to support the Mac OS defined folder for plugins which is what Apple is now stressing as a requirment for Mac OS X (the latest QuickTime installer puts the QT plugin there even under Mac OS 9). Since this is needed for Carbonization work I'd be happy to put it on my plate.

I didn't realize there were many entries per app key. We can't just get the "newest" or the one that was added last? I also didn't realize the Desktop DB wasn't neatly indexed so it takes a while. That's a bad idea and we really dont' need it. Steve, the Apple idea sounds interesting. Can you elaborate more or point us to an informative URL? Is it a simple system call to get the folder? This is only for "future" plugins though. What about using the IE plugins folder? I think those plugins should work in Mozilla too.

I haven't looked at DesktopDB code in ages but I think it's up to the caller to determine most recent if there are multiple instances of an app installed. The systemwide "Internet Plug-ins" folder can be found using FindFolder with the constant 'kInternetPlugInFolderType'. The same issue exists with multiple instances of an IE installation (and yes, the plugins installed with the Mac version of IE work in Netscape since we defined the plugin API for 'classic' plugins on the Mac OS). I'll also note that, under Mac OS X, only the global plugins folder is supported by IE so in some ways our life is greatly simplified by X.

Steve, that's great! This is no longer an [RFE] but a bug as we should definitly scan that folder! If only life was this easy on Windows.......

Peter, Rather than: mError = FindFolder(0, kInternetPlugInFolderType, true, & mSpec.vRefNum, & mSpec.parID); I'd suggest: mError = FindFolder(kOnAppropriateDisk, kInternetPlugInFolderType, kDontCreateFolder, & mSpec.vRefNum, &mSpec.parID); The headers don't indicate 0 as a valid vRefNum for FindFolder and we don't want to be creating the folder if it doesn't exist already.

nominating mozilla0.9.2 and clearning milestone. PDT: It would be nice to get this in for beta as most plugin installers don't recognize Mozilla but do find the system plug-ins folder. This would probably really enhance the user experience in Mozilla on the Mac as they can use the shared plugin folder (like IE) rather than re-installing all plugins.

peterl: line up your reviews and support from macdev (looks like some disagreement in this bug) and we'll talk. What's the risk of waiting for 0.9.2 to do this?

It's also a requirement to get plugins working under Mac OS X (more on that at MacDev today)

Doh!, over eager Commit on my last comment. I should clarify that all plugins under OS X should be in the system plugins folder rather than being in the app plugins folder.

I concur with sdagley's FindFolder change. Would also prefer '::FindFolder'. I'd also like to see some testing for the case where the FindFolder fails; we should fail gracefully.

Okay, looks like this is wanted, at least for mozilla0.9.2, so setting milestone. The patch I attached is just a WIP, and probably doesn't even compile but now that it's on my radar, I'll make the changes, test it, and get reviews.

beta stopper

This patch should now include the System's Internet Plug-ins folder "sweep" after the local one. At first, I seemed to have a problem with my Flash plugin that was already there. It wasn't being picked up at first because we died on opening the library. When I replaced it with the one in my Netscape folder, all was good. No other plugins showed weirdness. I'm seeking reviews....

Looks good. One thing - A directory called "Plugins" in the application directory has (AFAIK) never existed and, according to sdagley should be dropped. Can you drop the PLUGINS_DIR_LOCATION_MAC_OLD constant and the default case in the switch becomes "Plug-Ins"?

Also, can you replace uses of getApplicationSpec() with nsSpecialSystemDirectory appDir(Moz_BinDirectory) and then append to that? For embedding, we need to support configurations where Moz_BinDirectory != current process directory and using Moz_BinDirectory will allow that.

So what happens here if I have, say, Shockwave 8.5 r321 installed in my local folder and Shockwave 8.0 r21 installed in my "Internet Plug-ins" folder? I'd be surprised if we are handling this gracefully.

Looks great. Here's a suggestion you can take or leave: Why not just call PLUGINS_DIR_LOCATION_AUTO PLUGINS_DIR_LOCATION_APP. The AUTO part suggests something more than I think is going on. r=ccarlen

Peter, although not a bug in this patch, the call to IsSymLink will not work because that method (like many others) is broken on nsFileSpec.

I was a little hasty with that r=. IsSymLink isn't broken, it was being passed a bad spec. The reason it's bad is this: > nsSpecialSystemDirectory plugDir(nsSpecialSystemDirectory::Moz_BinDirectory); > *(nsFileSpec*)this = plugDir; > SetLeafName("Plug-ins"); That refers to a sibling of Moz_BinDir called "Plug-ins." You want it to be a child of Moz_BinDir with this: > nsSpecialSystemDirectory plugDir(nsSpecialSystemDirectory::Moz_BinDirectory); > *(nsFileSpec*)this = plugDir + "Plug-ins"; Also, we're scanning the app plug-in dir twice since the default arg to the nsPluginsDir constructor is PLUGINS_DIR_LOCATION_AUTO. First here: // scan Mozilla plugins dir nsPluginsDir pluginsDir; And then again here: nsPluginsDir pluginsDirMac(PLUGINS_DIR_LOCATION_AUTO);

Thanks Conrad for picking those up. I'll make the changes....but do you know what happenes in Brian Nesse's case of having one version of the plugin in one folder and another in the system folder.?

Looking at ScanPluginsDirectory, it's the first plugin encountered with a given file name that's used. I'm not sure whether we should improve this. Is newer better? Maybe, maybe not. If we did want to do something other than first come, first serve, we'd have to scan all dirs, collect the plugins in a temp array, then try to decide what to remove from that array (older versions of the same plug-in?) and only then, actually load them. This sounds like a lot of work for possibly no gain (the old plug-in was great, the new one sucks, who knows?)

There is more work to be done. If you visit a page that needs a plugin in the system's plugins folder, it fails. Conrad, looks like that "second scan" you were seeing was when a 4.x plugin was being created! It's ugly and this new patch builds on the uglyness, BUT, it does make Quicktime 5 (and now testing others) work out of the System's Internet Plug-ins Folder. I've gotten rid of using "AUTO" on Mac because it's too ambigious. Please review my hack and slash patch. Looks like mSpec.name isn't set correctly for the System folder's case, but I don't know if that matters.

My $.02 on the question of what to do if a plugin exists in both folders - use the one in the 'Internet Plug-ins' folder (i.e. make it the initial search folder). The logic here is that the system folder location is the new and preferred plugin location so if someone updates a plugin the odds are that it will be installed there first, especially once it's known that we support it.

That is indeed what QuickTime does. I wouldn't be surprised if others follow suit.

Bug #81659 is dependant on this fix and it's a PDT+'d bug so I'd assume that status extends to this checkin. That said, 2 comments on the latest patch: 1) Why would we ever not want to search the system plugins folder? I don't see a need for a pref to prevent it. 2) Per my previous comment, we should search the system plugin folder prior to the app local plugin folder.

I think I've addressed all issues in this next patch. System Folder is always scanned first. Please review and comment. I left the pref in there to make it easy to disable on the client side without ditching plugins all the plugins in the system folder. It doesn't hurt.

It would probably be good to add this to nsSpecialSystemDirectory, I'll file another bug or see if there is one.

From memory, I think Mac OS X has a user local Internet plugins folder too... (~/Library/Internet Plugins or somethign like that). It might be useful to include that one too (If it doesn't already).

Looks good. You fixed what I pointed out before. The part about scanning plug-in dirs for every 4x plug-in we find is terrible. That's not something you added in this patch, so I can't hold it against you. Please file a bug on that. r=ccarlen

tabbing looks messed up in ns4xPlugin.cpp changes, but otherwise sr=attinasi

a= asa@mozilla.org for checkin to frozen 0.9.2. (on behalf of drivers)

I'll give it an r= too but as Marcus Pallinger points out we also need to handle the user specific plugins folder on X (doh! thanks for the reminder Marcus :-) but this is definitely the right first step

One thing at a time. I've got to open a bunch of sub-bugs after I check this in, that'll be included. First, need to be able to build Carbon. Almost there.

Might it be that FindFolder(kOnAppropriateDisk, kInternetPlugInFolderType under OS X would return the eqivalent of that folder anyway? Also, until we can load the types of plug-ins we're likely to find there under OS X, not sure how much good its gonna do.

But with Patrick Beard's comment in bug 81659, he's got Quicktime working from the local plugins folder. It shouldn't be that much of a stretch for it to work in X.

Peter, that's what I meant, this code needs to go in then we can look at additional X issues.

Conrad, Under X we have the plug-ins folder, the system global Internet Plugins folder and the user specific Internet Plugins folder. Peter's change gets us the first 2 but unless X is doing some magic to make the global and user specific folders appear as one we're gonna need yet another search folder. That can come after we get the systemwide one supported.

Does Marcus' point about User-specific Internet Plug-In folders apply to Classic, also? I know that under Multiple Users certain System folders are duplicated inside the user's home directory (e. g., Startup Items and Preferences). Does it ever happen that a plug-in installer under OS 9 will get directed to such a place when run from a user account?

Patch checked in. Marking this bug FIXED. Please open sperate bugs on any problems this may cause. As promised, new bugs spawn: Bug 87226: Mac should NOT conduct a plugin folder scan in ns4xPlugin::CreatePlugin Bug 87227: nsSpecialSystemDirectory should include User and System "Plug-ins Folder" Bug 87228: Support for user "Plug-ins" folders on OS X Did I forget anything?

To address greg@tcp.com's question - the user specific plugin folder shouldn't be an issue under 9's Multiple User mode (if you're not the primary user I don't even believe you _can_ install software)

This does not work on OS X yet. The following patch removes the budle bit test and uses kLocalDomain. Seeking reviews.

+#if TARGET_CARBON + mError = ::FindFolder(kLocalDomain, kInternetPlugInFolderType, kDontCreateFolder, & mSpec.vRefNum, &mSpec.parID); +#else mError = ::FindFolder(kOnAppropriateDisk, kInternetPlugInFolderType, kDontCreateFolder, & mSpec.vRefNum, &mSpec.parID); - if (mError) +#endif This isn't good. Just because it's TARGET_CARBON does not mean we're running on OS X. Gestalt needs to be used to test the version of the system. Also, there's another bug on this - bug 87227, but this code should be moved to nsSpecialSystemDirectory.cpp and a key defined for it. There's already code in there which has things like kUserDomain conditionally defined depending on the version of UNIVERSAL_INTERFACES. We should keep this in one place.

Clearing rtm stopper and 0.9.2 graffiti. Still required for OSX however...

what's the status on this? we need this asap.

Per e-mail exchange, r=pinkerton Need a super review now.

You cannot use #if TARGET_CARBON to determine if you are running on X (since we may have Carbon Mac OS 9 builds one day). You need to do a runtime Gestalt test.

That was my complaint with the last patch but: ! #if TARGET_CARBON // on OS X, we must try kLocalDomain first ! mError = ::FindFolder(kLocalDomain, kInternetPlugInFolderType, kDontCreateFolder, & mSpec.vRefNum, &mSpec.parID); if (mError) + #endif + mError = ::FindFolder(kOnAppropriateDisk, kInternetPlugInFolderType, kDontCreateFolder, & mSpec.vRefNum, &mSpec.parID); + if (!mError) He's trying kLocalDomain under TARGET_CARBON first but, if that fails, as a Carbon build running under OS 9 will, the code continues on to use kOnAppropriateDisk.

doh, I should have pasted the e-mail here. Simon, can you check this again? That last patch gives us the best of both worlds without using Gestalt. The first ::FindFolder (using kLocalDomain) works in the TARGET_CARBON block because of the |if| statment after it. So.... * In OS X this should give us what we want. * In OS 9 Carbon, this WILL fail but then the second ::FindFolder will use the correct constant. * In OS 9 Classic, the first ::FindFolder won't be there.

What if kLocalDomain resolves to some other, unintended folder on 9? Or what if Apple change things so that it does in future? I think doing it like this is dangerous. Calling Gestalt is only 2 lines of code. E.g. in nsThemeHandler.cpp: 272 static bool isMacOSXOrLater() 273 { 274 static long version = 0; 275 if (version == 0) 276 Gestalt(gestaltSystemVersion, &version); 277 return (version >= 0x1000); 278 }

I needed to add kLocalDomain in order to compile this. Yuck. I'll say again: Also, there's another bug on this - bug 87227, but this code should be moved to nsSpecialSystemDirectory.cpp and a key defined for it. There's already code in there which has things like kUserDomain conditionally defined depending on the version of UNIVERSAL_INTERFACES. We should keep this in one place. In the meanwhile...

I applied the 07/06 patch and we now seem to load the QuickTime plugin under X. Unfortunately it doesn't seem to draw anything correctly (test page is quicktime.apple.com) and will crash if you actually try to play a movie.

This is the same problem Brian Nesse and I have been trying to fix all day. I guess it's all Mac, not just Classic. Win32 seems to work. The plugin seems unwilling to do anything even though it seems to be returning success on all the functions I've looked at. The is downloaded, and playable in the cache. The one thing which strikes me sas odd is that leaving the page doesn't seem to call |~ns4xPluginInstance|. However, that could be unrelated this and more related to another bug. Does Flash work on Carbon?

Not quite sure if there is already a bug open for Quicktime not working in Classic, but this one-line patch fixes the problem. Brian and I were looking at this all day Friday but it was me who had incorrectly placed ::UseResFile() in the new order of calls in bug 85334. I'm seeking review and super review for patch 41563. If you want me to open a seperate bug and get a PDT+, let me know.

Also, more along the lines of this bug, I finally got a Carbon build working and tested out Condrad's patch for OS X plugins: http://bugzilla.mozilla.org/showattachment.cgi?attach_id=41476 With this patch, I see Quicktime working in Fizilla! However, Flash still does not work :( I'll open a seperate bug on that as it works with IE. Conrad, try to get patch #41476 checked-in, r=peterl

Simon, can you sr= attachment 41476 [details] [diff] [review]? Peter - we need this on branch and trunk?

sr=sfraser

Attachment attachment 41476 [details] [diff] [review] needs to go on both the branch and the trunk.

Reassign to Conrad.

Regarding patch 41563 for the classic fix. Shouldn't the same fix also apply for carbon (i.e. opening the resfile *after* creating the 4xplugin)?. I'm a little concerned by all the #ifdef TARGET_CARBON differentiation in the Mac code between what's happening in CreatePlugin and what's happening in the ns4xPlugin constructor. Shouldn't this code be the same except for the PR_FindSymbol call?

Accepting, updating status.

Where did my comment go? Brian, it was done that way because it when I had it the other way, Quicktime didn't work. Taking for Conrad for checkin tonight.

Patch in trunk and branch, marking FIXED.

this worksnow.