231337 - Clean up old Mac OS X runtime tests

Status: RESOLVED FIXED

(Camino Graveyard :: General, defect)

Description

[Josh Aas]

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.

Comment 1

[Josh Aas]

Attachment: 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.

Comment 2

[Josh Aas]

patch is good - compiles fine, no bustage in my testing. pretty straightforward

Comment 3

[Boris Zbarsky [:bzbarsky]]

I'm not likely to get to this anytime soon (next 2 weeks, probably).

Comment 4

[Mike Pinkerton (not reading bugmail)]

cc'ing bryner because he told me on irc yesterday that he was going to do this,
or had a patch, or something.

Comment 5

[Steve Dagley]

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?

Comment 6

[Josh Aas]

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.

Comment 7

[Josh Aas]

This patch needs some whitespace cleanup, and perhaps the gestalt.h does not
need to be included in some of the files... coming up

Comment 8

[Josh Aas]

Attachment: 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.

Comment 9

[Boris Zbarsky [: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).

Comment 10

[Josh Aas]

Attachment: 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.

Comment 11

[Josh Aas]

Attachment: 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.

Comment 12

[Josh Aas]

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: ----------------------------------------------------------------------

Comment 13

[Boris Zbarsky [:bzbarsky]]

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.

Comment 14

[Josh Aas]

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.

Comment 15

[Christian :Biesinger (don't email me, ping me on IRC)]

LDAP makefiles are mangled by the build system when not using a separate objdir.
don't know about nfspwd.

Comment 16

[Josh Aas]

really landed this time