Last Comment Bug 281889 - remove XP_MAC code
: remove XP_MAC code
Status: RESOLVED FIXED
[See comment 75]
:
Product: Core
Classification: Components
Component: General (show other bugs)
: Trunk
: PowerPC Mac System 9.x
: -- minor (vote)
: ---
Assigned To: Josh Aas
:
Mentors:
http://mxr.mozilla.org/comm-central/s...
: 196105 208982 248039 280247 (view as bug list)
Depends on: 309392 224012 400188 400605 460310 461911 461912 461913 476996 479480 480745 495228 509533 655756
Blocks: 283572
  Show dependency treegraph
 
Reported: 2005-02-10 20:39 PST by Josh Aas
Modified: 2013-12-27 14:29 PST (History)
17 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
cleanup v1.0 (40.54 KB, patch)
2005-02-10 20:39 PST, Josh Aas
no flags Details | Diff | Splinter Review
xpcom cleanup v1.1 (40.54 KB, patch)
2005-02-10 21:52 PST, Josh Aas
no flags Details | Diff | Splinter Review
js cleanup v1.0 (37.10 KB, patch)
2005-02-10 21:52 PST, Josh Aas
no flags Details | Diff | Splinter Review
layout cleanup v1.0 (24.68 KB, patch)
2005-02-10 22:16 PST, Josh Aas
no flags Details | Diff | Splinter Review
js cleanup v2.0 (35.99 KB, patch)
2005-04-16 03:06 PDT, Ludovic Hirlimann [:Usul]
no flags Details | Diff | Splinter Review
remove UNIVERSAL_INTERFACES_VERSION macros, some XP_MAC [Checked in: Comment 21] (19.90 KB, patch)
2005-07-03 20:22 PDT, Josh Aas
sfraser_bugs: review+
bryner: superreview+
benjamin: approval1.8b3+
Details | Diff | Splinter Review
updated patch to remove XP_MAC from js (39.04 KB, patch)
2005-07-07 15:32 PDT, Josh Aas
no flags Details | Diff | Splinter Review
(Dv1a) <xpfe/*> (Sync./Moved from bug 196105) (16.18 KB, patch)
2005-07-10 05:30 PDT, Serge Gautherie (:sgautherie)
no flags Details | Diff | Splinter Review
browser component, also fix XP_MACSOX bug v1.0 (2.87 KB, patch)
2005-10-25 13:53 PDT, Josh Aas
asaf: review+
Details | Diff | Splinter Review
nsObjectFrame and some plugin cleanup v1.0 [Checked in: Comment 40] (10.50 KB, patch)
2005-11-06 16:30 PST, Josh Aas
mark: review+
sfraser_bugs: superreview+
Details | Diff | Splinter Review
plugin cleanup (13.59 KB, patch)
2006-07-13 11:01 PDT, Josh Aas
mark: review+
mikepinkerton: superreview+
Details | Diff | Splinter Review
fix v1.0 (10.44 KB, patch)
2006-11-15 11:33 PST, Josh Aas
no flags Details | Diff | Splinter Review
network fix v1.0.1 (11.32 KB, patch)
2006-11-15 11:38 PST, Josh Aas
darin.moz: review+
Details | Diff | Splinter Review
mail fix v1.0 (4.21 KB, patch)
2006-11-15 14:43 PST, Josh Aas
no flags Details | Diff | Splinter Review
mailnews fix v1.0 (48.23 KB, patch)
2006-11-15 16:23 PST, Josh Aas
no flags Details | Diff | Splinter Review
xpcom fix v1.0 (71.09 KB, patch)
2006-11-16 01:11 PST, Josh Aas
no flags Details | Diff | Splinter Review
xpinstall fix v1.0 (13.78 KB, patch)
2006-11-16 12:53 PST, Josh Aas
dveditz: review+
mikepinkerton: superreview+
Details | Diff | Splinter Review
nsExternalHelperAppService fix v1.0 (7.97 KB, patch)
2007-03-05 10:07 PST, Josh Aas
bzbarsky: review+
bzbarsky: superreview+
Details | Diff | Splinter Review
more mailnews v1.0 (13.72 KB, patch)
2007-07-14 16:40 PDT, Phil Ringnalda (:philor, back in August)
mozilla: review+
mozilla: superreview+
Details | Diff | Splinter Review
more mailnews, part deux v1.0 (2.91 KB, patch)
2007-07-15 19:55 PDT, Phil Ringnalda (:philor, back in August)
mozilla: review+
mozilla: superreview+
Details | Diff | Splinter Review
more layout,content v1.0 (22.48 KB, patch)
2007-10-01 10:36 PDT, Josh Aas
mozcbarrett: review+
roc: superreview+
roc: approval1.9+
Details | Diff | Splinter Review
Fix/update a (/mailnews/) case mismatch (v1) (644 bytes, patch)
2007-10-21 08:56 PDT, Serge Gautherie (:sgautherie)
mozilla: review-
mozilla: superreview-
Details | Diff | Splinter Review
Remove a (/mailnews/) case mismatch (v2) [Checkin: comment 83] (577 bytes, patch)
2007-10-22 10:24 PDT, Serge Gautherie (:sgautherie)
mozilla: review+
mozilla: superreview+
Details | Diff | Splinter Review

Description Josh Aas 2005-02-10 20:39:10 PST
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.
Comment 1 Josh Aas 2005-02-10 20:39:46 PST
Created attachment 174016 [details] [diff] [review]
cleanup v1.0
Comment 2 Josh Aas 2005-02-10 21:52:11 PST
Created attachment 174020 [details] [diff] [review]
xpcom cleanup v1.1
Comment 3 Josh Aas 2005-02-10 21:52:45 PST
Created attachment 174021 [details] [diff] [review]
js cleanup v1.0
Comment 4 Josh Aas 2005-02-10 22:16:34 PST
Created attachment 174023 [details] [diff] [review]
layout cleanup v1.0
Comment 5 Ludovic Hirlimann [:Usul] 2005-02-11 11:27:54 PST
xpcom is taken care in another bug.
Comment 6 Ludovic Hirlimann [:Usul] 2005-02-11 11:28:40 PST
Comment on attachment 174020 [details] [diff] [review]
xpcom cleanup v1.1

Geoff no need to review this one. see bug 280247
Comment 7 Ludovic Hirlimann [:Usul] 2005-02-11 11:33:43 PST
Comment on attachment 174023 [details] [diff] [review]
layout cleanup v1.0

asking review. We don't build Mac since 1.3.
Comment 8 Ludovic Hirlimann [:Usul] 2005-02-11 11:52:16 PST
As per Irc discussion asking r to brendan fro the js part and ccing shaver.
Comment 9 Boris Zbarsky [:bz] 2005-02-11 12:29:26 PST
So have drivers generally OK'ed this?
Comment 10 Ludovic Hirlimann [:Usul] 2005-02-11 12:48:25 PST
brendan I posted your question in <1gru8hk.j17n9af4h7awN%ludonews@nerim.net>.
Comment 11 Ludovic Hirlimann [:Usul] 2005-02-11 13:11:45 PST
(In reply to comment #9)
> So have drivers generally OK'ed this?

as of right now, no.
Comment 12 Brendan Eich [:brendan] 2005-02-11 15:18:42 PST
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
Comment 13 Ludovic Hirlimann [:Usul] 2005-02-11 22:40:06 PST
(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
Comment 14 Boris Zbarsky [:bz] 2005-02-15 21:26:04 PST
So basically, I'd like to hear that this idea has driver approval before I spend
time reviewing any code changes...
Comment 15 Boris Zbarsky [:bz] 2005-02-20 22:45:14 PST
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...
Comment 16 Asa Dotzler [:asa] 2005-02-22 10:08:27 PST
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. 
Comment 17 Ludovic Hirlimann [:Usul] 2005-04-16 03:06:34 PDT
Created attachment 180868 [details] [diff] [review]
js cleanup v2.0
Comment 18 Josh Aas 2005-07-03 20:22:12 PDT
Created attachment 188164 [details] [diff] [review]
remove UNIVERSAL_INTERFACES_VERSION macros, some XP_MAC
[Checked in: Comment 21]

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 19 Josh Aas 2005-07-05 01:23:47 PDT
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 20 Benjamin Smedberg AWAY UNTIL 2-AUG-2016 [:bsmedberg] 2005-07-05 14:11:08 PDT
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 21 Josh Aas 2005-07-05 18:24:44 PDT
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
Comment 22 Josh Aas 2005-07-07 15:32:18 PDT
Created attachment 188600 [details] [diff] [review]
updated patch to remove XP_MAC from js
Comment 23 Josh Aas 2005-07-07 15:36:21 PDT
checked in patch to remove all XP_MAC from js
Comment 24 Josh Aas 2005-07-08 15:23:39 PDT
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.
Comment 25 Josh Aas 2005-07-08 15:24:21 PDT
*** Bug 280247 has been marked as a duplicate of this bug. ***
Comment 26 Josh Aas 2005-07-08 15:25:06 PDT
*** Bug 248039 has been marked as a duplicate of this bug. ***
Comment 27 Josh Aas 2005-07-08 15:26:03 PDT
*** Bug 196105 has been marked as a duplicate of this bug. ***
Comment 28 Serge Gautherie (:sgautherie) 2005-07-10 05:30:49 PDT
Created attachment 188836 [details] [diff] [review]
(Dv1a) <xpfe/*> (Sync./Moved from bug 196105)

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 29 Simon Fraser 2005-07-20 13:48:03 PDT
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.
Comment 30 Serge Gautherie (:sgautherie) 2005-07-20 14:08:55 PDT
(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...
Comment 31 Celso Aguiar 2005-09-20 15:12:56 PDT
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.
Comment 32 Boris Zbarsky [:bz] 2005-09-20 15:24:23 PDT
Filed bug 309392 on that (so we can request the right blocking flags).
Comment 33 Josh Aas 2005-10-25 13:53:09 PDT
Created attachment 200789 [details] [diff] [review]
browser component, also fix XP_MACSOX bug v1.0
Comment 34 Mano (::mano, needinfo? for any questions; not reading general bugmail) 2005-10-25 16:20:14 PDT
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.
Comment 35 Josh Aas 2005-10-25 16:29:27 PDT
"browser component, also fix XP_MACSOX bug v1.0" landed on trunk
Comment 36 Josh Aas 2005-11-06 16:30:00 PST
Created attachment 202051 [details] [diff] [review]
nsObjectFrame and some plugin cleanup v1.0
[Checked in: Comment 40]
Comment 37 Mark Mentovai 2005-11-09 13:57:08 PST
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.
Comment 38 Josh Aas 2005-11-09 14:45:06 PST
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 39 Mark Mentovai 2005-11-09 19:26:12 PST
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++?
Comment 40 Josh Aas 2005-11-10 09:46:16 PST
landed "nsObjectFrame and some plugin cleanup v1.0" on the trunk with comments taken into account
Comment 41 Erik Fabert 2006-03-18 11:52:07 PST
*** Bug 208982 has been marked as a duplicate of this bug. ***
Comment 42 Josh Aas 2006-07-13 11:01:44 PDT
Created attachment 229123 [details] [diff] [review]
plugin cleanup
Comment 43 Josh Aas 2006-07-13 11:02:31 PDT
note - the plugin cleanup patch includes a comment change to indicate that nsplugin.h does not in fact supercede npapi.h.
Comment 44 Josh Aas 2006-07-13 11:11:44 PDT
Comment on attachment 229123 [details] [diff] [review]
plugin cleanup

there aren't any actual code changes here, just going straight for sr
Comment 45 Mark Mentovai 2006-07-13 12:04:23 PDT
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 46 Mike Pinkerton (not reading bugmail) 2006-07-13 12:07:09 PDT
Comment on attachment 229123 [details] [diff] [review]
plugin cleanup

sr=pink
Comment 47 Josh Aas 2006-07-13 12:47:56 PDT
plugin cleanup landed on trunk
Comment 48 Josh Aas 2006-11-15 11:33:14 PST
Created attachment 245682 [details] [diff] [review]
fix v1.0

Going straight to sr since this is simple.
Comment 49 Josh Aas 2006-11-15 11:38:59 PST
Created attachment 245684 [details] [diff] [review]
network fix v1.0.1

missed a file
Comment 50 Josh Aas 2006-11-15 13:42:48 PST
network fix v1.0.1 checked in on trunk
Comment 51 Josh Aas 2006-11-15 14:43:33 PST
Created attachment 245696 [details] [diff] [review]
mail fix v1.0

has r from bienvenu, landed on trunk
Comment 52 Josh Aas 2006-11-15 16:23:29 PST
Created attachment 245703 [details] [diff] [review]
mailnews fix v1.0

r=bienvenu, landed on trunk
Comment 53 Josh Aas 2006-11-16 01:11:58 PST
Created attachment 245745 [details] [diff] [review]
xpcom fix v1.0

r=bsmedberg, landed on trunk
Comment 54 Josh Aas 2006-11-16 12:53:45 PST
Created attachment 245780 [details] [diff] [review]
xpinstall fix v1.0
Comment 55 Daniel Veditz [:dveditz] 2006-11-27 10:16:27 PST
Comment on attachment 245780 [details] [diff] [review]
xpinstall fix v1.0

r=dveditz
Comment 56 Mike Pinkerton (not reading bugmail) 2006-11-27 10:24:00 PST
Comment on attachment 245780 [details] [diff] [review]
xpinstall fix v1.0

rs=pink
Comment 57 Josh Aas 2006-11-27 10:48:05 PST
"xpinstall fix v1.0" landed on trunk
Comment 58 Josh Aas 2007-03-05 10:07:19 PST
Created attachment 257382 [details] [diff] [review]
nsExternalHelperAppService fix v1.0
Comment 59 Boris Zbarsky [:bz] 2007-03-05 21:12:21 PST
Comment on attachment 257382 [details] [diff] [review]
nsExternalHelperAppService fix v1.0

r+sr=bzbarsky
Comment 60 Josh Aas 2007-03-05 21:17:20 PST
"nsExternalHelperAppService fix v1.0" landed on trunk
Comment 61 Phil Ringnalda (:philor, back in August) 2007-07-14 16:40:37 PDT
Created attachment 272355 [details] [diff] [review]
more mailnews v1.0
Comment 62 David :Bienvenu 2007-07-14 16:49:42 PDT
Comment on attachment 272355 [details] [diff] [review]
more mailnews v1.0

thx, Phil
Comment 63 Phil Ringnalda (:philor, back in August) 2007-07-14 17:44:57 PDT
"more mailnews v1.0" landed on trunk
Comment 64 Phil Ringnalda (:philor, back in August) 2007-07-15 19:55:11 PDT
Created attachment 272437 [details] [diff] [review]
more mailnews, part deux v1.0

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 65 David :Bienvenu 2007-07-15 21:18:57 PDT
Comment on attachment 272437 [details] [diff] [review]
more mailnews, part deux v1.0

thx, Phil.
Comment 66 Phil Ringnalda (:philor, back in August) 2007-07-15 21:59:29 PDT
"more mailnews, part deux v1.0" landed on the trunk
Comment 67 Josh Aas 2007-10-01 10:36:06 PDT
Created attachment 283027 [details] [diff] [review]
more layout,content v1.0
Comment 68 Colin Barrett [:cbarrett] 2007-10-01 10:56:04 PDT
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")
Comment 69 Josh Aas 2007-10-01 11:12:47 PDT
As for the stuff about event processing, those are separate bugs I plan to file once I have time to test more.
Comment 70 Josh Aas 2007-10-01 16:21:00 PDT
"more layout,content v1.0" checked in
Comment 71 Mike Schroepfer 2007-10-17 12:27:56 PDT
Josh - should this bug still be open?
Comment 72 Phil Ringnalda (:philor, back in August) 2007-10-17 13:04:32 PDT
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.
Comment 73 Josh Aas 2007-10-17 18:37:31 PDT
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.
Comment 74 Brian Crowder 2007-10-17 19:42:49 PDT
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.
Comment 75 Josh Aas 2007-10-17 20:33:08 PDT
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.
Comment 76 Steffen Wilberg 2007-10-18 00:25:38 PDT
Uh, but who's going to file all those bugs, let alone fix them? Who cares enough?
Comment 77 Serge Gautherie (:sgautherie) 2007-10-21 08:37:39 PDT
Comment on attachment 188836 [details] [diff] [review]
(Dv1a) <xpfe/*> (Sync./Moved from bug 196105)

Moved to bug 400605.
Comment 78 Serge Gautherie (:sgautherie) 2007-10-21 08:56:45 PDT
Created attachment 285660 [details] [diff] [review]
Fix/update a (/mailnews/) case mismatch (v1)

Fixes the one line which is not found by the URL (case sensitive) query.
Comment 79 David :Bienvenu 2007-10-21 16:59:29 PDT
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...
Comment 80 Serge Gautherie (:sgautherie) 2007-10-22 10:24:06 PDT
Created attachment 285747 [details] [diff] [review]
Remove a (/mailnews/) case mismatch (v2)
[Checkin: comment 83]

Per comment 79.
Comment 81 David :Bienvenu 2007-10-22 10:31:56 PDT
Comment on attachment 285747 [details] [diff] [review]
Remove a (/mailnews/) case mismatch (v2)
[Checkin: comment 83]

thx for the patch
Comment 82 David :Bienvenu 2007-10-22 17:18:34 PDT
adding checkin-needed keyword
Comment 83 Reed Loden [:reed] (use needinfo?) 2007-10-22 17:53:01 PDT
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

Note You need to log in before you can comment on or make changes to this bug.