287944 - Rebrand bootstrap's OS2 bits

Status: RESOLVED FIXED

(SeaMonkey :: UI Design, defect)

Description

[Ian Neal]

See list below:
/xpfe/bootstrap/nsNativeAppSupportOS2.cpp, line 293 -- * Mozilla registers the
DDE application "Mozilla" and currently
/xpfe/bootstrap/nsNativeAppSupportOS2.cpp, line 820 -- #define
MOZ_DDE_APPLICATION "Mozilla"
/xpfe/bootstrap/nsNativeAppSupportOS2.cpp, line 1549 -- url.Insert( "mozilla
-url ", 0 );
/xpfe/bootstrap/nsNativeAppSupportOS2.cpp, line 1719 -- url.Insert( "mozilla
-url ", 0 );
/xpfe/bootstrap/nsNativeAppSupportOS2.cpp, line 1950 -- // window when given
"mozilla -foobar" or the like.
/xpfe/bootstrap/splashos2.rc, line 56 -- ID_DDE_APPLICATION_NAME, "Mozilla"
/xpfe/bootstrap/os2turbo/mozturbo.cpp, line 158 -- printf("Mozilla for OS/2
preloader\n"\
/xpfe/bootstrap/os2turbo/mozturbo.cpp, line 177 -- printf("Mozilla for OS/2
preloader is not running\n");
/xpfe/bootstrap/os2turbo/mozturbo.cpp, line 198 -- printf("Mozilla for OS/2
preloader is already running\n");

Comment 1

[Robert Kaiser]

Attachment: part of a patch, only things that I felt remotely sure about :)

OK, this patch should be a base to build upon. I didn't build it and fixed
mostly comments, along with the DDE name (be sure to have my current rebranding

patch in, else you won't get the MOZ_APP_DISPLAYNAME from Makefile.in!).

I was preparing this as part of my rebranding patch, but then noticed this bug
is around.
That patch also gives you #defines for MOZ_APP_DISPLAYNAME (earlier "Mozilla")
and MOZ_APP_NAME (the binary's name, earlier "mozilla") so you can use those in

thise file as well.

Those are the bits I was still unsure of and didn't touch:

mozilla/xpfe/bootstrap/nsNativeAppSupportOS2.cpp:1549:			 
url.Insert( "mozilla -url ", 0 );
mozilla/xpfe/bootstrap/nsNativeAppSupportOS2.cpp:1719:		  url.Insert(
"mozilla -url ", 0 );
mozilla/xpfe/bootstrap/nsNativeAppSupportOS2.cpp:1950:	  // window when given
"mozilla -foobar" or the like.
mozilla/xpfe/bootstrap/os2turbo/mozturbo.cpp:38:This program implements a
module preloader for the OS/2 version of the Mozilla
mozilla/xpfe/bootstrap/os2turbo/mozturbo.cpp:47:The list of module names was
determined by loading Mozilla and then
mozilla/xpfe/bootstrap/os2turbo/mozturbo.cpp:158:    printf("Mozilla for OS/2
preloader\n"\
mozilla/xpfe/bootstrap/os2turbo/mozturbo.cpp:177:    printf("Mozilla for OS/2
preloader is not running\n");
mozilla/xpfe/bootstrap/os2turbo/mozturbo.cpp:198:      printf("Mozilla for OS/2
preloader is already running\n");
mozilla/xpfe/bootstrap/splashos2.rc:56:    ID_DDE_APPLICATION_NAME,	      
"Mozilla"

Comment 2

[Peter Weilbacher]

What's the point of changing all instances of "Mozilla" in the comments to
"Seamonkey", even before a final name is decided? From the discussion in the
newsgroup I don't think that MoFo requested to change that. Better postpone this
until the name is final?!

As for the registered DDE name, I think it will cause problems with the OS/2
applications that expect that name. Even in toolkit it remains "Mozilla", the
same seems to be true for Windows as well. The printf strings will be changed
easily.

Comment 3

[Peter Weilbacher]

Attachment: replaces relevant Mozilla's in OS/2 bootstrap and mozturbo code

This patch changes all Mozilla instances in nsNativeAppSupportOS2.cpp and
mozturbo.cpp in comments to SeaMonkey, but leaves the DDE name as "Mozilla" and
commandline examples in comments as mozilla.exe. It also removes some warnings
for mozturbo (I stripped spaces from line endings after backslashes). The
printfs and url.Inserts should now reflect the MOZ_APP_* defines. The patch
compiles, but where do I change the MOZ_APP_* defines to see if it really works
for different names?

What's supposed to happen to the name of mozturbo.exe?

Comment 4

[Robert Kaiser]

(In reply to comment #2)
> As for the registered DDE name, I think it will cause problems with the OS/2
> applications that expect that name. Even in toolkit it remains "Mozilla", the
> same seems to be true for Windows as well.

We're changing the DDE string to MOZ_APP_DISPLAYNAME on Windows. If you don't in
OS/2, it's your choice :)

(In reply to comment #3)
> Created an attachment (id=179372) [edit]
> replaces relevant Mozilla's in OS/2 bootstrap and mozturbo code

Looks good to me from what I can tell (and I don't know OS/2 or C++ code too much).

> where do I change the MOZ_APP_* defines to see if it really works
> for different names?

Those are set in the configure script.

> What's supposed to happen to the name of mozturbo.exe?

Not sure (this is OS/2-only, right?), but either rename it to
$(MOZ_APP_NAME)-turbo.exe or leave it as-is, I'd say.

Comment 5

[Robert Kaiser]

Peter:
For bug 285696 I checked in a change that goes without adding quotes to the
variables, please use that style as well for the os2turbo makefile and make the
.cpp file use NS_STRINGIFY()
Just look at the patch I checked in for bug 287943 for how it should be done.

Comment 6

[Peter Weilbacher]

Attachment: 2nd try

Robert, I don't think we can use NS_STRINGIFY in mozturbo. This is not
dependent on XPCOM, and making it dependent on that would defeat this as a
"turbo" loader. I therefore left the double quotes in the Makefile as well.
But I changed nsNativeAppSupportOS2.cpp accordingly. I also removed the unused
MOZ_DDE_APPLICATION as you did on Windows, the real DDE appname is defined in
splashos2.rc and remains Mozilla.

Mike, can you review, and perhaps even test?

Comment 7

[Peter Weilbacher]

Comment on attachment 181186 [details] [diff] [review]
2nd try

I now tested this, it seems to work for the parts it changes when setting the
variables in configure to something else (like "Seamonkey"). But I continue to
get Mozilla everywhere in the UI, where do I have to change that?

Comment 8

[Peter Weilbacher]

(In reply to comment #7)
> But I continue to get Mozilla everywhere in the UI, where do I 
> have to change that?

Ignore that last comment, of course this also works after editing brand.dtd.

Comment 9

[Robert Kaiser]

> Ignore that last comment, of course this also works after editing brand.dtd.

This and other stuff will be done shortly with a "stage 2" patch in bug 285696 ;-)

Comment 10

[Robert Kaiser]

The main rebranding bits have landed, it would be nice to support OS/2 again as
well... 

Comment 11

[Peter Weilbacher]

Sure, and the patch is there and works fine for me. 

Any suggestion on how to proceed with regard to reviews? Mike Kaply doesn't seem
to have enough time to spend on OS/2 reviews...

Comment 12

[Robert Kaiser]

do we have anyone else who could revie OS/2 stuff? pedemonte or anyone?

Comment 13

[Mike Kaply [:mkaply]]

Comment on attachment 181186 [details] [diff] [review]
2nd try

r=mkaply

Comment 14

[Robert Kaiser]

mkaply, can we mark that FIXED with your latest checkins?

(BTW, many thanks for doing that stuff!)

Comment 15

[Peter Weilbacher]

Comment on attachment 181186 [details] [diff] [review]
2nd try

Not yet, at least I don't see that anything of this patch got checked in yet.

Comment 16

[Robert Kaiser]

Comment on attachment 181186 [details] [diff] [review]
2nd try

move a? to 1.8b4

Comment 17

[Mike Kaply [:mkaply]]

Fix checked in.