Since lots of the Mac code in Mozilla was written for Mac OS 9 first, when Mac OS X added there were sometimes runtime checks used to determine whether Mac OS X was being run. Now, since any time Mozilla code is run on a Mac it must be Mac OS X and therefore those checks are unnecessary. This patch cleans out lots of old code and elimintes the wasteful checks.
Created attachment 139326 [details] [diff] [review] patch v1.0 I'll mark this ready for review when I'm done testing and cleaning for sure... just here for my organizational purposes now.
patch is good - compiles fine, no bustage in my testing. pretty straightforward
I'm not likely to get to this anytime soon (next 2 weeks, probably).
cc'ing bryner because he told me on irc yesterday that he was going to do this, or had a patch, or something.
Comment on attachment 139326 [details] [diff] [review] patch v1.0 This reminds me we've got a lot of #if TARGET_CARBON in the code. Any plans to change that?
Attachment #139326 - Flags: review?(sdagley) → review+
I plan on taking care of the carbon ifdefs sometime in the next couple months, but I figured I'd get the runtime stuff done first so we don't waste time on calls to gestalt and whatnot.
This patch needs some whitespace cleanup, and perhaps the gestalt.h does not need to be included in some of the files... coming up
Created attachment 140481 [details] [diff] [review] patch v1.1 (whitespace cleanup) The only difference between this patch and the first one is replacing some tabs with the correct number of spaces.
Attachment #139326 - Attachment is obsolete: true
14 years ago
Attachment #139326 - Flags: superreview?(bzbarsky)
Comment on attachment 140481 [details] [diff] [review] patch v1.1 (whitespace cleanup) >Index: xpfe/appshell/src/nsAppShellService.cpp >@@ -1017,37 +1009,4 @@ > takeAction = PR_TRUE; > } >- if (!takeAction) >- return NS_OK; Can't you remove the code up to this point too? It seems like this whole method is really OS9-only.... sr=bzbarsky with this issue addressed one way or another (either removing all the unused code here or explaining why it can't be removed).
Attachment #140481 - Flags: superreview?(bzbarsky) → superreview+
Created attachment 163008 [details] [diff] [review] updated patch This patch removes the method bz commented about altogether. also, the patch is updated to the current trunk code. Not requesting a review yet since I haven't tested it. Just putting the patch here so I can keep track of it.
Attachment #140481 - Attachment is obsolete: true
Created attachment 163101 [details] [diff] [review] good version of patch Landed this patch. This patch is the same as the one bz sr'd but it is updated for the current trunk (removed some changes already made on the trunk) and addresses his comment by removing the rest of the code in the method.
Attachment #163008 - Attachment is obsolete: true
Anyone know why when I try to land this patch, cvs picks up a bunch of files under the "Tag: ldapcsdk_50_client_branch" heading that I don't mean to modify? CVS: ---------------------------------------------------------------------- CVS: Enter Log. Lines beginning with `CVS:' are removed automatically CVS: CVS: Committing in . CVS: CVS: Modified Files: CVS: Tag: ldapcsdk_50_client_branch CVS: directory/c-sdk/config/Makefile directory/c-sdk/config/nfspwd CVS: directory/c-sdk/ldap/clients/tools/Makefile CVS: directory/c-sdk/ldap/libraries/libiutil/Makefile CVS: directory/c-sdk/ldap/libraries/libldif/Makefile CVS: directory/c-sdk/ldap/libraries/libssldap/Makefile CVS: No tag CVS: gfx/src/mac/nsNativeThemeMac.cpp CVS: gfx/src/mac/nsRenderingContextMac.cpp CVS: gfx/src/mac/nsRenderingContextMac.h CVS: xpfe/appshell/src/nsAppShellService.cpp CVS: ----------------------------------------------------------------------
Because you're not running 'cvs commit' on just the files you want to commit? Also, you did notice that the trunk is closed due to mac bustage, right? Making checkins right now is a bad idea; doing it in mac code is a really bad idea.
I didn't land the patch due to the cvs commit funniness. I've never had "cvs commit" pick up anything but my intended changes before. Also I should have noticed the bustage - my bad, won't happen again.
LDAP makefiles are mangled by the build system when not using a separate objdir. don't know about nfspwd.
really landed this time
Status: NEW → RESOLVED
Last Resolved: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.