Closed
Bug 195148
Opened 21 years ago
Closed 21 years ago
XPInstall API's getFolder should support Classic and OSX folders under OSX
Categories
(Core Graveyard :: Installer: XPInstall Engine, enhancement)
Tracking
(Not tracked)
VERIFIED
FIXED
People
(Reporter: arun, Assigned: ssu0262)
References
Details
(Whiteboard: [adt2])
Attachments
(2 files, 4 obsolete files)
47.41 KB,
patch
|
Details | Diff | Splinter Review | |
801 bytes,
application/x-xpinstall
|
Details |
Currently, on Mac OS X, the getFolder method (getFolder: http://devedge.netscape.com/library/manuals/2001/xpinstall/1.0/Chap211.html#1012570 ) can be invoked with a few special strings to return the locations of special folders such as "Plugins", "Components" and a few other platform specific ones on Mac OS, such as "Mac Preferences". But performing installations to the Global Plugins directory (/Library/Internet Plug-ins), including for XPT files as well as actual plugin files, is becoming desirable, especially on OS like Mac where there aren't two distinct binary architectures for plugins (ActiveX and NPAPI). Thus, one XPInstall could benefit multiple browsers if the support is there. So this RFE is asking for some syntactic sugar that returns the global plugins directory (say something like getFolder("Mac Plugins")), especially if the user has permission to install there. If not, they can harness the error condition and install to the user-specific plugins directory (or whatever getFolder("Plugins") points to).
Comment 1•21 years ago
|
||
There is the backend support for this in directory service. See the key NS_APP_PLUGINS_DIR_LIST. It returns a list of directories. They are are (on OSX anyway): Plug-ins in the app bundle ~/Library/Internet Plug-ins /Library/Internet Plug-ins That set and its order is not frozen, but could be. Maybe we could say that the order of the folders returned is ranked in terms of size of domain over which it's shared (the opposite of the above list). Or, permissions restrictions.
Comment 2•21 years ago
|
||
installer triage: nsbeta1+ adt2 still need bugs filed for the other target folders that are currently disabled/#ifdef'ed out on Mac OS X for the getFolder() function.
Keywords: nsbeta1+
Whiteboard: adt2
Reporter | ||
Comment 3•21 years ago
|
||
I suggested above (when I logged this bug) that you could call getFolder("file:///", subPath) to get to the HD::Library::Internet Plug-ins location. But actually, that's not always true. In fact, some drives are called Macintosh HD but what if the drive was called Arun OS X HD ? Essentially, what I mean is, you can never tell what the volume name is going to be. So I use this technique: var subPath = "../Library/Internet Plug-ins"; var globalPluginsPath = getFolder("Mac System", subPath); Then to add to /Library/Internet Plug-ins, I use an addFile invocation on globalPluginsPath. This is a bit of a hack, and we ought to have a better way, but I'm posting this here so that someone can see a workaround. I don't think there's a way to use Install.gestalt( ) either. Of course, it's a different matter that I ran into bug 195377 .
Comment 4•21 years ago
|
||
> var globalPluginsPath = getFolder("Mac System", subPath);
Don't do that. That only works on XP_MAC (CFM) builds, which we no longer make.
"Mac System" means the Mac Classic system folder which, on an OSX system,
doesn't have to exist, or may be on a different drive from "/"
A better way is to ask directory service for NS_APP_PLUGINS_DIR_LIST, which
gives back an nsISimpleEnumerator, and use the last item. That will be
/Library/Internet Plug-ins. Because of permissions of the current user, you
really want to enumerate backwards over the list of returned plugin folders
until success. Oh well, with an nsISimpleEnumerator, that'll take some work :-/
Reporter | ||
Comment 5•21 years ago
|
||
ccarlen, I suspected that my workaround would only apply to CFM builds (e.g. N7.02) since the System X directory need not exist. The purpose of this RFE was to expose a much better way via XPInstall. What you propose isn't workable in today's XPInstall unless I'm mistaken.
Updated•21 years ago
|
Whiteboard: adt2 → [adt2]
Updated•21 years ago
|
Severity: normal → enhancement
Summary: RFE: XPInstall API's getFolder should return the /Library/Internet Plug-ins directory also → XPInstall API's getFolder should return the /Library/Internet Plug-ins directory also
initial patch that will now support the following folders under OSX: Classic folders ** Mac System : /System Folder/ ** Mac Desktop : null ** Mac Trash : null ** Mac Startup : /System Folder/Startup Items/ ** Mac Shutdown : /System Folder/Shutdown Items/ ** Mac Apple Menu : /System Folder/Apple Menu Items/ ** Mac Control Panel : /System Folder/Control Panels/ ** Mac Extension : /System Folder/Extensions/ ** Mac Fonts : /System Folder/Fonts/ ** Mac Preferences : /System Folder/Preferences/ ** Mac Documents : null OSX folders ** Temporary : /private/tmp/9299/Temporary Items/ ** MacOSX Home : /Users/[user]/ ** MacOSX Default Download : /Users/[user]/Desktop/ ** MacOSX User Desktop : /Users/[user]/Desktop/ ** MacOSX Local Desktop : null ** MacOSX User Applications : /Users/[user]/Applications/ ** MacOSX Local Applications : /Applications/ ** MacOSX User Documents : /Users/[user]/Documents/ ** MacOSX Local Documents : null ** MacOSX User Internet PlugIn : /Users/[user]/Library/Internet Plug-Ins/ ** MacOSX Local Internet PlugIn: /Library/Internet Plug-Ins/ ** MacOSX User Frameworks : null ** MacOSX Local Frameworks : /Library/Frameworks/ ** MacOSX User Preferences : /Users/[user]/Library/Preferences/ ** MacOSX Local Preferences : /Library/Preferences/ ** MacOSX Picture Documents : /Users/[user]/Pictures/ ** MacOSX Movie Documents : /Users/[user]/Movies/ ** MacOSX Internet Sites : /Users/[user]/Sites/
Trigger this .xpi file to test the new osx folder functions. After installation is complete, look at install.log for output.
updating summary to reflect fix.
Status: NEW → ASSIGNED
Summary: XPInstall API's getFolder should return the /Library/Internet Plug-ins directory also → XPInstall API's getFolder should support Classic and OSX folders under OSX
Attachment #119205 -
Flags: superreview?(dveditz)
Attachment #119205 -
Flags: review?(ccarlen)
Comment 9•21 years ago
|
||
Comment on attachment 119205 [details] [diff] [review] patch v1.0 >Index: xpcom/io/SpecialSystemDirectory.cpp >=================================================================== > #if defined(XP_MAC) > //---------------------------------------------------------------------------------------- > static nsresult nsIFileFromOSType(OSType folderType, nsILocalFile** aFile) > //---------------------------------------------------------------------------------------- > { > CInfoPBRec cinfo; > DirInfo *dipb=(DirInfo *)&cinfo; > > // hack: first check for kDefaultDownloadFolderType > // if it is, get Internet Config Download folder, if that's > // not availble use desktop folder > if (folderType == kDefaultDownloadFolderType) > { > nsCOMPtr<nsIInternetConfigService> icService (do_GetService(NS_INTERNETCONFIGSERVICE_CONTRACTID)); > if (icService) > { > if (NS_SUCCEEDED(icService->GetDownloadFolder(&mSpec))) > return; > else > folderType = kDesktopFolderType; >@@ -823,20 +847,21 @@ > // The "create" parameter, however, is sadly ignored. > // How do we internationalize this? > return nsIFileFromOSType(kVolumeRootFolderType; > *this += "Documents"; > CreateDirectory(); > break; > } // switch > } // for > StrFileName filename; > filename[0] = '\0'; > dipb->ioNamePtr = (StringPtr)&filename; > dipb->ioFDirIndex = -1; > > mError = NS_FILE_RESULT(PBGetCatInfoSync(&cinfo)); > if (NS_SUCCEEDED(mError)) > { > mError = NS_FILE_RESULT(FSMakeFSSpec(dipb->ioVRefNum, dipb->ioDrParID, filename, &mSpec)); > } > } > #endif // XP_MAC >+ This code wouldn't compile if XP_MAC was on. Can you just chop it out while you're there? >Index: xpcom/io/SpecialSystemDirectory.h >=================================================================== > nsILocalFile** aFile); >+NS_IMETHODIMP >+GetOSXFolderType(short aDomain, OSType aFolderType, nsILocalFile **localFile); > Since this isn't an nsI method impl, use an nsresult >Index: xpcom/io/nsDirectoryServiceDefs.h >=================================================================== >- #define NS_MAC_CLASSICPREFS_DIR "ClassicPrfs" Make sure to do a full build of the tree - I think this may be used in profile mgr and that'll need to be changed to NS_MAC_PREFS_DIR. And then, not from the patch: ** MacOSX User Frameworks : null These few which are null - I suspect it's because they never exist (?) Can you look into that and, if they never will exist, get rid of them? Symmetry is nice, but... Otherwise - excellent!
Assignee | ||
Comment 10•21 years ago
|
||
this patch addresses conrad's comments: * removed obsolete XP_MAC code that doesn't compile * fixed return type of GetOSXFolderType() * searched for all other occurrances of NS_MAC_CLASSICPREFS_DIR and replaced it with NS_MAC_PREFS_DIR * 'MacOSX User Frameworks' no longer returns 'null'. 'Local Desktop' and 'Local Documents' still return null, but they exist. Could be an insufficient access problem. I'm leaving it in the patch in case the user is able to get the appropriate permissions for access.
Attachment #119205 -
Attachment is obsolete: true
Attachment #119205 -
Flags: superreview?(dveditz)
Attachment #119205 -
Flags: review?(ccarlen)
Attachment #119222 -
Flags: review?(ccarlen)
Comment 11•21 years ago
|
||
Comment on attachment 119222 [details] [diff] [review] patch v1.1 r=ccarlen
Attachment #119222 -
Flags: review+
Attachment #119222 -
Flags: superreview?(dveditz)
Reporter | ||
Comment 12•21 years ago
|
||
Looks like the following things will happen that I'm interested in(ssu/ccarlen, correct me if wrong): 1. To install to the app. bundle's plugins directory, getFolder("Plugins") will work as usual. 2. To install to the user's plugins directory, getFolder("MacOSX User Internet PlugIn") will now work instead of any hack. 3. To install to the Global Plugins directory, getFolder("MacOSX Local Internet PlugIn") will now work instead of any hack. There is a potential fourth installation directory, namely what is specified in the OS X Un*x shell via an environment variable, but let's not worry about that. This pretty much meets my requirements when I logged the bug, namely that the API should offer robust accessibility to other locations on the machine for plugins to be installed to. I'm CC'ing peterl for completeness.
Comment 13•21 years ago
|
||
Comment on attachment 119222 [details] [diff] [review] patch v1.1 sr=sspitzer (moving r= to dveditz, in case you wanted him to see this as well.) two minor questions / comments: 1) was nothing needed for Mac OS X? Or will cavin be back in here, when he trying to fix 4.x migration for Mac OS X? #if defined(XP_OS2) NS_GetSpecialDirectory(NS_OS2_DIR, getter_AddRefs(oldRegFile)); #elif defined(XP_WIN) NS_GetSpecialDirectory(NS_WIN_WINDOWS_DIR, getter_AddRefs(oldRegFile)); #elif defined(XP_MAC) - NS_GetSpecialDirectory(NS_MAC_CLASSICPREFS_DIR, getter_AddRefs(oldRegFile)); + NS_GetSpecialDirectory(NS_MAC_PREFS_DIR, getter_AddRefs(oldRegFile)); #endif #endif 2) I haven't seen the surrounding code, but I bet maybe this code be made smaller. looks like a lot of copy paste. I bet it could be written to have this code once: + nsCOMPtr<nsIProperties> directoryService = + do_GetService(NS_DIRECTORY_SERVICE_CONTRACTID, &rv); + if (!directoryService) return; + directoryService->Get(XXX, + NS_GET_IID(nsIFile), + getter_AddRefs(mFileSpec)); and use the switch statement, to determine XXX from the switch value. if you agree, feel free to spin that off to a code cleanup bug.
Attachment #119222 -
Flags: superreview?(dveditz)
Attachment #119222 -
Flags: superreview+
Attachment #119222 -
Flags: review?(dveditz)
Attachment #119222 -
Flags: review?(ccarlen)
Comment 14•21 years ago
|
||
Comment on attachment 119222 [details] [diff] [review] patch v1.1 this work will collide with the work alec is doing. please talk with him before landing. I do not think that you want to define this globally - please wrap it in a #ifdef. +nsresult +GetOSXFolderType(short aDomain, OSType aFolderType, nsILocalFile **localFile);
Comment 15•21 years ago
|
||
> was nothing needed for Mac OS X? Or will cavin be back in here, when he trying to fix 4.x migration for Mac OS X? Yes, something is needed for that code but it's not part of this bug. Once this is checked in, it will be easy to use what this patch gives to enable 4.x migration for Mach-O. BTW, I have bug 190336. Didn't know we had decided to fix that or not.
Assignee | ||
Comment 16•21 years ago
|
||
to address seth's comment #2, bug 125106 will take care of optimizations when I merge its patch to the trunk. I'll attach a new patch given dougt's comment.
Assignee | ||
Comment 17•21 years ago
|
||
this patch has been verified to compile under OSX, Linux, and Win32.
Attachment #119222 -
Attachment is obsolete: true
Assignee | ||
Comment 18•21 years ago
|
||
This is simply an updated patch that has been merged with trunk. I ran my .xpi test file under OSX and everything is still working. This is the patch that I just checked in.
Attachment #119497 -
Attachment is obsolete: true
Assignee | ||
Comment 19•21 years ago
|
||
closing bug as fixed.
Status: ASSIGNED → RESOLVED
Closed: 21 years ago
Resolution: --- → FIXED
Assignee | ||
Comment 20•21 years ago
|
||
the old .xpi file was missing the test for the User Music folder. This one has it.
Attachment #119206 -
Attachment is obsolete: true
Comment 21•21 years ago
|
||
Trunk build 2003-04-07: Mac 10.1.5 I just noticed with this build that the Import Mail dialog (or Import Settings dialog) no longer displays Comm 4.x as an option. In fact this dialog is blank. Could this be a regression due to this fix? I checked trunk build 2003-04-04 and it atleast lists the Comm 4.x option.
Comment 22•21 years ago
|
||
> I just noticed with this build that the Import Mail dialog (or Import Settings
> dialog) no longer displays Comm 4.x as an option
can you start a new bug, and assign to cavin, who is looking into mac os x issues.
Comment 23•21 years ago
|
||
> can you start a new bug, and assign to cavin, who is looking into mac os x issues. I spun it off to bug #201157
Assignee | ||
Comment 24•21 years ago
|
||
I just installed Monday's build and from mail, I selected Tools|Import. For Address Books and Mail, I see 'Communicator 4.x' listed after I click the "Next" button from the 1st dialog. Is this what you're referring to that isn't working? If not, can you tell me how to get at the dialog that you're seeing the problem with?
Comment 25•21 years ago
|
||
There is a problem but not exactly how I described in comment# 21. I updated bug# 201157 which explains in more detail. I don't believe that a recent regression occured. Sorry for the false alarm.
Comment 26•21 years ago
|
||
verified fixed build 2003041410 (thanks for test file) also XPInstall getfolder tests showing same results as test file :)
Status: RESOLVED → VERIFIED
Comment 27•21 years ago
|
||
Comment on attachment 119222 [details] [diff] [review] patch v1.1 clearing review request.
Attachment #119222 -
Flags: review?(dveditz)
Updated•9 years ago
|
Product: Core → Core Graveyard
You need to log in
before you can comment on or make changes to this bug.
Description
•