Closed Bug 337324 Opened 16 years ago Closed 16 years ago
move Firefox-specific microsummary directory defs to ns
Browser Directory Provider
Microsummary directories are defined in nsDirectoryServiceDefs, but on IRC bsmedberg says "we have nsBrowserDirectoryProvider for firefox-specific keys,and the #define should be in a firefox-specific place, either in some nsMicroSummaries.h or nsBrowserCompsCID.h".
Target Milestone: --- → Firefox 2 beta1
This patch moves the microsummary directories to nsBrowserDirectoryProvider. For filename consistency with nsDirectoryServiceDefs.h and nsAppDirectoryServiceDefs.h, I put the microsummary defines into the new file nsBrowserDirectoryServiceDefs.h. I also copied nsAppFileLocationProvider::CloneMozBinDirectory() into nsBrowserDirectoryProvider. And I fixed a hardcoded reference to the UsrMicsumDefs directory in nsMicrosummaryService.js.in. Do I still need to provide a Mac OS 9-specific (i.e. XP_MAC) name for the microsummary generators directory, given that Firefox no longer supports that OS?
myk, I don't think you want the copying-from-defaults behavior of the lower section of browserdirectory, so I remade your patch to shortcut everything in the first set of if/else blocks. You don't need CloneMozBinDirectory, you can safely just use NS_XPCOM_CURRENT_PROCESS_DIR
> myk, I don't think you want the copying-from-defaults behavior of the lower > section of browserdirectory, so I remade your patch to shortcut everything in > the first set of if/else blocks. You don't need CloneMozBinDirectory, you can > safely just use NS_XPCOM_CURRENT_PROCESS_DIR Yeah, that all seems right to me and has the corollary benefit of making the logic simpler. This patch looks good, except that we probably don't need to do #if defined(XP_MAC), since browser doesn't support Mac OS 9-, and it almost certainly never will. Also, we don't need to #include nsILocalFile.h now that we aren't adding CloneMozBinDirectory. Here's an updated patch with these changes.
Attachment #224913 - Flags: approval-branch-1.8.1?(mconnor)
Attachment #224913 - Flags: approval-branch-1.8.1?(mconnor) → approval-branch-1.8.1+
Fix checked in to trunk and 1.8 branch.
Status: ASSIGNED → RESOLVED
Closed: 16 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.