It'll make it easier to read and get rid of some Mac OS X runtime code if we clean up the XP_MAC stuff in XPCOM.

xpcom is taken care in another bug.

Comment on attachment 174020 [details] [diff] [review] xpcom cleanup v1.1 Geoff no need to review this one. see bug 280247

Comment on attachment 174023 [details] [diff] [review] layout cleanup v1.0 asking review. We don't build Mac since 1.3.

As per Irc discussion asking r to brendan fro the js part and ccing shaver.

So have drivers generally OK'ed this?

brendan I posted your question in <1gru8hk.j17n9af4h7awN%ludonews@nerim.net>.

(In reply to comment #9) > So have drivers generally OK'ed this? as of right now, no.

Ludovic, I couldn't find your post in news://news.mozilla.org/netscape.public.mozilla.jseng -- can you give a google groups link to it? /be

(In reply to comment #12) > Ludovic, I couldn't find your post in > news://news.mozilla.org/netscape.public.mozilla.jseng -- can you give a google > groups link to it? > > /be http://groups-beta.google.com/group/netscape.public.mozilla.jseng/msg/198d36139ddcad7a

So basically, I'd like to hear that this idea has driver approval before I spend time reviewing any code changes...

Comment on attachment 174023 [details] [diff] [review] layout cleanup v1.0 Please re-request review once it's clear that this is the policy we want to be following...

It sounds like there may be some modules that still want to support Mac OS 9. This will probably require individual module owner approvals from the affected areas.

All versions of Mac OS X that we support define UNIVERSAL_INTERFACES_VERSION to be 0x0400, so ifdefs using that can be removed. This patch also removes some large XP_MAC section. Patch prompted by finding lots of clutter in lxr. I've tested buildng Firefox with this patch and it works fine. Very safe.

Comment on attachment 188164 [details] [diff] [review] remove UNIVERSAL_INTERFACES_VERSION macros, some XP_MAC [Checked in: Comment 21] asking for approval since this is a very safe patch - doesn't mess with code that we actually compile

Comment on attachment 188164 [details] [diff] [review] remove UNIVERSAL_INTERFACES_VERSION macros, some XP_MAC [Checked in: Comment 21] a=bsmedberg, please land expeditiously

Comment on attachment 188164 [details] [diff] [review] remove UNIVERSAL_INTERFACES_VERSION macros, some XP_MAC [Checked in: Comment 21] this patch was landed, marking it obsolete to remove clutter

checked in patch to remove all XP_MAC from js

All XP_MAC removal stuff should go in this bug. Closing 248039, 280247, 281889. Please put component and patch version number in patch attachment names.

*** Bug 280247 has been marked as a duplicate of this bug. ***

*** Bug 248039 has been marked as a duplicate of this bug. ***

*** Bug 196105 has been marked as a duplicate of this bug. ***

I have no compiler: Could you compile/test/(super-)review/check in this patch ? Thanks. {{ Index: mozilla/xpfe/components/autocomplete/src/nsLDAPAutoCompleteSession.cpp -// build system. The MOZ_LDAP_XPCOM preprocessor symbol is only -// defined on Mac because noone else needs this weirdness; thus }} I wonder if |MOZ_LDAP_XPCOM| could be removed too (other bug) then ? {{ (from bug 196105) ------- Additional Comment #21 From Serge GAUTHERIE 2005-02-06 14:21 PDT [reply] ------- (In reply to comment #20) > (From update of attachment 173538 [details] [diff] [review] [edit] [edit]) > Why is there no XP_MACOSX code in showOSAlert.cpp? I would have no idea: I'm not a Mac user, and I assume that the current code works as it is/was. Should these XP_MAC be replaced by XP_MACOSX, instead of removed ? Any hint are/will welcomed: "helpwanted". }}

Comment on attachment 188836 [details] [diff] [review] (Dv1a) <xpfe/*> (Sync./Moved from bug 196105) Index: mozilla/xpfe/bootstrap/nsAppRunner.cpp =================================================================== RCS file: /cvsroot/mozilla/xpfe/bootstrap/nsAppRunner.cpp,v retrieving revision 1.441 diff -u -p -r1.441 nsAppRunner.cpp --- mozilla/xpfe/bootstrap/nsAppRunner.cpp 30 Apr 2005 12:39:25 -0000 1.441 +++ mozilla/xpfe/bootstrap/nsAppRunner.cpp 10 Jul 2005 11:42:18 -0000 // nsISplashScreen will be removed. // -#if !defined(XP_WIN) && !defined(XP_OS2)&& !defined( XP_BEOS ) && !defined(MOZ_WIDGET_GTK) && !defined(MOZ_WIDGET_GTK2) && !defined(XP_MAC) && (!defined(XP_MACOSX) || defined(MOZ_WIDGET_COCOA)) +#if !defined(XP_WIN) && !defined(XP_OS2) && !defined(XP_BEOS) && !defined(MOZ_WIDGET_GTK) && !defined(MOZ_WIDGET_GTK2) && (!defined(XP_MACOSX) || defined(MOZ_WIDGET_COCOA)) Can we turn this #ifdef around, and test for platforms that DO want this? nsresult NS_CreateNativeAppSupport(nsINativeAppSupport **aResult) { @@ -395,10 +362,6 @@ static int TranslateReturnValue(nsresult return 1; } Index: mozilla/xpfe/bootstrap/showOSAlert.cpp =================================================================== RCS file: /cvsroot/mozilla/xpfe/bootstrap/showOSAlert.cpp,v retrieving revision 1.10 diff -u -p -r1.10 showOSAlert.cpp --- mozilla/xpfe/bootstrap/showOSAlert.cpp 20 Aug 2004 15:01:40 -0000 1.10 +++ mozilla/xpfe/bootstrap/showOSAlert.cpp 10 Jul 2005 11:42:18 -0000 @@ -43,9 +43,6 @@ //defines and includes for previous installation cleanup process #if defined (XP_WIN) #include <windows.h> -#elif defined (XP_MAC) -#include <Dialogs.h> -#include <TextUtils.h> #elif defined (MOZ_WIDGET_GTK) #include <gtk/gtk.h> #elif defined (XP_OS2) @@ -195,9 +192,6 @@ printf("\n****Inside ShowOSAlert ***\n") #if defined (XP_WIN) MessageBox(NULL, message_copy, NULL, MB_OK | MB_ICONERROR | MB_SETFOREGROUND ); -#elif (XP_MAC) - short buttonClicked; - StandardAlert(kAlertStopAlert, c2pstr(message_copy), nil, nil, &buttonClicked); #elif defined (MOZ_WIDGET_GTK) NS_gtk_alert(message_copy, NULL, "OK"); #elif defined (XP_OS2) Is there a Mac OS X block here at all? Not sure if this is used, but we might want it. Index: mozilla/xpfe/components/bookmarks/src/nsBookmarksService.h =================================================================== RCS file: /cvsroot/mozilla/xpfe/components/bookmarks/src/nsBookmarksService.h,v retrieving revision 1.39 diff -u -p -r1.39 nsBookmarksService.h --- mozilla/xpfe/components/bookmarks/src/nsBookmarksService.h 17 Apr 2004 16:51:21 -0000 1.39 +++ mozilla/xpfe/components/bookmarks/src/nsBookmarksService.h 10 Jul 2005 11:42:20 -0000 @@ -63,7 +63,7 @@ class nsIOutputStream; #ifdef DEBUG -#if defined(XP_MAC) || defined(XP_MACOSX) +#if defined(XP_MACOSX) #include <Timer.h> #endif #endif Probably don't need Timer.h any more.

(In reply to comment #29) > (From update of attachment 188836 [details] [diff] [review] [edit]) Thanks for your comments. Helpwanted: looking for someone with a MacOSX compiler to answer/test...

Changes made to prmjtime.c, related to this bug, have broken Mac Date object, try this on Win or Mac: new Date().getTimezoneOffset() On Win it returns 420, on Mac 0.

Filed bug 309392 on that (so we can request the right blocking flags).

Comment on attachment 200789 [details] [diff] [review] browser component, also fix XP_MACSOX bug v1.0 >Index: components/bookmarks/src/nsBookmarksService.h >=================================================================== >-#if defined(XP_MAC) || defined(XP_MACOSX) >+#if defined(XP_MACOSX) >Index: components/migration/src/nsDogbertProfileMigrator.h >=================================================================== > >-#if defined(XP_MAC) || defined(XP_MACOSX) >+#if defined(XP_MACOSX) make those |#ifdef XP_MACOSX|. r=mano.

"browser component, also fix XP_MACSOX bug v1.0" landed on trunk

Comment on attachment 202051 [details] [diff] [review] nsObjectFrame and some plugin cleanup v1.0 [Checked in: Comment 40] --- layout/generic/nsObjectFrame.cpp 4 Nov 2005 02:38:32 -0000 1.531 +++ layout/generic/nsObjectFrame.cpp 6 Nov 2005 23:58:55 -0000 [...] @@ -3090,7 +3086,7 @@ nsresult nsPluginInstanceOwner::DispatchFocusToPlugin(nsIDOMEvent* aFocusEvent) { -#if !(defined(XP_MAC) || defined(XP_MACOSX)) +#if !defined(XP_MACOSX) Use #ifndef. @@ -3209,7 +3205,7 @@ nsresult nsPluginInstanceOwner::KeyPress(nsIDOMEvent* aKeyEvent) { -#if defined(XP_MAC) || defined(XP_MACOSX) // send KeyPress events only on Mac +#if defined(XP_MACOSX) // send KeyPress events only on Mac Use #ifdef. There are many other locations in this file that you're changing that should use #ifndef and #ifdef, too. --- modules/plugin/base/src/nsPluginHostImpl.cpp 20 Oct 2005 00:28:35 -0000 1.537 +++ modules/plugin/base/src/nsPluginHostImpl.cpp 6 Nov 2005 23:59:01 -0000 @@ -4729,7 +4729,7 @@ // On Windows, we also want to include the Quicktime plugin from the 4.x directory // But because it spans several DLL's, the best check for now is by filename - if (nsnull != PL_strcasestr(tag->mFileName,"npqtplugin")) + if (PL_strcasestr(tag->mFileName,"npqtplugin") != nsnull) We've got constant-first comparisons elsewhere in that file, why are you only changing this one? All or none, I think, unless you're working with a specific area of code. --- modules/plugin/base/src/nsPluginsDirDarwin.cpp 7 Aug 2005 07:54:50 -0000 1.8 +++ modules/plugin/base/src/nsPluginsDirDarwin.cpp 6 Nov 2005 23:59:05 -0000 @@ -53,19 +53,16 @@ #include "nsPluginsDirUtils.h" #include "nsILocalFileMac.h" -#include <Carbon/Carbon.h> + #include <string.h> #include <stdio.h> #include <unistd.h> #include <fcntl.h> + +#include <Carbon/Carbon.h> #include <mach-o/loader.h> #include <mach-o/fat.h> -#include <CFURL.h> -#include <CFBundle.h> -#include <CFString.h> -#include <CodeFragments.h> Although Carbon/Carbon.h eventually brings in CoreFoundation headers, it feels wrong to me. I'd be happier here if you added CoreServices/CoreServices.h to the mix. These are trivial changes, I'd say you can make them on checkin.

For the constant-first check, that is just there because I did it on a whim. Not necessary, but no good reason to take it out of the patch. As for #ifndef, I don't like using that because it looks too much like #ifdef. The equivalent sytax I used makes it more clear at a glance. I'll add the corefoundation include on checkin. Thanks for the review.

Comment on attachment 202051 [details] [diff] [review] nsObjectFrame and some plugin cleanup v1.0 [Checked in: Comment 40] r+ is still valid after reading your comments upon my comments. Does that make r++?

landed "nsObjectFrame and some plugin cleanup v1.0" on the trunk with comments taken into account

*** Bug 208982 has been marked as a duplicate of this bug. ***

note - the plugin cleanup patch includes a comment change to indicate that nsplugin.h does not in fact supercede npapi.h.

Comment on attachment 229123 [details] [diff] [review] plugin cleanup there aren't any actual code changes here, just going straight for sr

Comment on attachment 229123 [details] [diff] [review] plugin cleanup >Index: public/nsplugin.h >- * <P>This superscedes the old plugin API (npapi.h, npupp.h), and >+ * This interface was an attempt to supercede npapi.h, npupp.h but it failed. It Fix spelling: supersede.

Comment on attachment 229123 [details] [diff] [review] plugin cleanup sr=pink

plugin cleanup landed on trunk

Going straight to sr since this is simple.

missed a file

network fix v1.0.1 checked in on trunk

has r from bienvenu, landed on trunk

r=bienvenu, landed on trunk

r=bsmedberg, landed on trunk

Comment on attachment 245780 [details] [diff] [review] xpinstall fix v1.0 r=dveditz

Comment on attachment 245780 [details] [diff] [review] xpinstall fix v1.0 rs=pink

"xpinstall fix v1.0" landed on trunk

Comment on attachment 257382 [details] [diff] [review] nsExternalHelperAppService fix v1.0 r+sr=bzbarsky

"nsExternalHelperAppService fix v1.0" landed on trunk

Comment on attachment 272355 [details] [diff] [review] more mailnews v1.0 thx, Phil

"more mailnews v1.0" landed on trunk

Having missed these would be less embarrassing if not for the fact that one of them is the file that made me notice them, by triggering a compiler warning about trigraphs not being enabled.

Comment on attachment 272437 [details] [diff] [review] more mailnews, part deux v1.0 thx, Phil.

"more mailnews, part deux v1.0" landed on the trunk

Comment on attachment 283027 [details] [diff] [review] more layout,content v1.0 Two nits: 1) if it were me, I would leave the |#pragma mark -|s in, because they add a horizontal separator to the list of functions in my editor, making it easier to find things. Up to you though. (Xcode also behaves this way fwiw) 2) File a bug about those comments that say "mac can't process events while dragging", since we can do that now (I know you asked steven in IRC, but just file a bug so it doesn't get completely lost) Otherwise, r+. Good to get this out of the tree. (perhaps for moz2 we can move back to just "mac" instead of MAC_OS_X and "cocoa")

As for the stuff about event processing, those are separate bugs I plan to file once I have time to test more.

"more layout,content v1.0" checked in

Josh - should this bug still be open?

The URL field says "yes, it should be open, 558 times," and the fact that cairo-platform.h is one of the things listed says it should probably be higher priority, to get the dead ones out so we can see newer mistakes - since cairo's vastly more recent than our System 9 support, that seems likely to have intended to be active for XP_MACOSX but isn't.

Mike - either we leave this open and keep using it as a catch-all, or we just have people file individual bugs for component-specific patches from here on out. I don't care either way, at this point I don't see a compelling reason to change what we're doing with this bug. If anyone feels strongly the other way, fine with me. Just comment here giving a reason and if it makes sense I'll close this out.

I think the general trend of smaller bugs for individual issues is more widely followed on BMO and functions more effectively overall. If we're voting, I'd vote to close this one or evolve it to a meta-bug with smaller bugs depending on it.

Fine with me. I don't want to do the meta bug thing though, there is really no point to keeping track of them collectively. The "problem" of remaining XP_MAC is small enough at this point that we hardly care any more anyway. So - if your patch just removes XP_MAC from a component file a new bug from now on.

Uh, but who's going to file all those bugs, let alone fix them? Who cares enough?

Comment on attachment 188836 [details] [diff] [review] (Dv1a) <xpfe/*> (Sync./Moved from bug 196105) Moved to bug 400605.

Fixes the one line which is not found by the URL (case sensitive) query.

Comment on attachment 285660 [details] [diff] [review] Fix/update a (/mailnews/) case mismatch (v1) the comment describes the change that was made - it was correct...if you want to remove the comment, that's OK, but changing it isn't right...

Per comment 79.

Comment on attachment 285747 [details] [diff] [review] Remove a (/mailnews/) case mismatch (v2) [Checkin: comment 83] thx for the patch

adding checkin-needed keyword

Checking in mailnews/compose/src/nsMsgAppleDoubleEncode.cpp; /cvsroot/mozilla/mailnews/compose/src/nsMsgAppleDoubleEncode.cpp,v <-- nsMsgAppleDoubleEncode.cpp new revision: 1.26; previous revision: 1.25 done