281375 - [BEOS] Implement toolkit/xre and xpfe/bootstrap native app support

Status: RESOLVED FIXED

(SeaMonkey :: General, defect, P2)

Description

[tqh]

User-Agent:       Mozilla/5.0 (BeOS; U; BeOS BePC; en-US; rv:1.8b) Gecko/20050202 Firefox/1.0+
Build Identifier: 

BeOS is currently contaminating nsAppRunner with code that should live in the
nativeAppSupport. Nobody wants that and moving it to where it belongs help me do
a lot of clever stuff (TM).

I will provide patches for this shortly.

Reproducible: Always

Comment 1

[tqh]

Attachment: Untested patch for xpfe/bootstrap, see comment.

This is the changes I had in mind for Mozilla. It's not tested since my Mozilla
quits in nsAppStartup->doProfileSomething (with or without this patch). So I'm
going to get a new tree, which is about time anyway.

Comment 2

[tqh]

Seems to work fine.

Comment 3

[tqh]

Attachment: Removes BeOS code from nsAppRunner and puts it in nsAppSupportBeOS (toolkit and xpfe)

Comment 4

[tqh]

Attachment: The nsNativeAppSupportBeOS.cpp needed for toolkit/xre

Comment 5

[tqh]

Comment on attachment 173982 [details] [diff] [review]
Removes BeOS code from nsAppRunner and puts it in nsAppSupportBeOS (toolkit and xpfe)

r?

Comment 6

[tqh]

Attachment: Separating xpfe and toolkit - this is xpfe

multidir patch was hard to apply because of same filenames.

Comment 7

[tqh]

Attachment: Same for toolkit/xre

Comment 8

[tqh]

Comment on attachment 173982 [details] [diff] [review]
Removes BeOS code from nsAppRunner and puts it in nsAppSupportBeOS (toolkit and xpfe)

no need to review this one.

Comment 9

[Sergei Dolgov]

per attachment https://bugzilla.mozilla.org/attachment.cgi?id=174101

Code functionality is ok, but i prefer we use this case to unify
nsNativeSupportBeOS.cpp style

In some places ident is 2 spaces, in some - 3, in some - 4.

also there is mix of

foo {
}
and

foo
{
}

and also mix of styles for logical expressions in if and for and so on.

sometimes it is
if (foo)
sometimes
if( foo ).

Comment 10

[Sergei Dolgov]

about Firefox patch.
Same notices as for Mozilla.

And one additional thing i'm unsure about - 
in Makefile you use test for (*TOOLKIT, beos)
but in past i got notices that i should use test for OS instead.

Comment 11

[Sergei Dolgov]

Comment on attachment 173983 [details]
The nsNativeAppSupportBeOS.cpp needed for toolkit/xre

patch will get review after reformatting

Comment 12

[Sergei Dolgov]

Comment on attachment 174101 [details] [diff] [review]
Separating xpfe and toolkit - this is xpfe

patch will get review after reformatting

Comment 13

[Sergei Dolgov]

Comment on attachment 174102 [details] [diff] [review]
Same for toolkit/xre

patch will get review after reformatting

Comment 14

[tqh]

Attachment: Tried to clean up formatting

Cleaned up patch, not tested though as I currently only have Firefox-tree.
Should be ok though.

Comment 15

[tqh]

Attachment: nsNativeAppSupportBeOS.cpp for xre

cleaned up.

Comment 16

[tqh]

Comment on attachment 174102 [details] [diff] [review]
Same for toolkit/xre

This one is ok I think, conforming to others in Makefile.in

Comment 17

[Sergei Dolgov]

Comment on attachment 175035 [details]
nsNativeAppSupportBeOS.cpp for xre

r=sergei_d

Comment 18

[Sergei Dolgov]

Comment on attachment 175032 [details] [diff] [review]
Tried to clean up formatting

r=sergei_d

Comment 19

[Sergei Dolgov]

Comment on attachment 174102 [details] [diff] [review]
Same for toolkit/xre

r=sergei_d

Comment 20

[tqh]

Comment on attachment 175032 [details] [diff] [review]
Tried to clean up formatting

Could you sr (and maybe commit) the xpfe changes?

Comment 21

[tqh]

Comment on attachment 174102 [details] [diff] [review]
Same for toolkit/xre

As this one touches nsAppRunner and Makefile.in   I'm asking for sr.

(I guess nsNativeAppSupportBeOS.cpp is ok as is.)

Comment 22

[Benjamin Smedberg]

Comment on attachment 174102 [details] [diff] [review]
Same for toolkit/xre

no sr needed for port-specific code if it is reviewed by the proper owner.

Comment 23

[tqh]

Comment on attachment 175032 [details] [diff] [review]
Tried to clean up formatting

sr?
(also need someone to check this in)

Comment 24

[tqh]

Comment on attachment 174102 [details] [diff] [review]
Same for toolkit/xre

r?

(moves out BeOS code from nsAppRunner and to new nsNativeAppSupportBeOS.cpp)

Comment 25

[tqh]

Need a checkin, tried asking on CVS which only resulted in some unwarrented
stupid remarks.

Anyway, all files should have been reviewed properly (nsNativeAppSupportBeOS is
BeOS only). It's a high priority for me as it hinders me from reworking
nsToolkit/nsAppShell to work better for embedding and to allow better
integration with OS (NativeAppSupport).

Comment 26

[Sergei Dolgov]

"tried asking on CVS"

Do you mean IRC? #developers channel?

Comment 27

[timeless]

Comment on attachment 175035 [details]
nsNativeAppSupportBeOS.cpp for xre

i was about to check this in, but i read this file and suspect that this block
is wrong, i'd rather someone clear it up before i commit:

> * The Original Code is Mozilla Communicator client code.
> *
> * The Initial Developer of the Original Code is
> * Netscape Communications Corporation.
> * Portions created by the Initial Developer are Copyright (C) 1998
> * the Initial Developer. All Rights Reserved.
> *
> * Contributor(s):
> *   Fredrik Holmqvist <[e-mail]>

Comment 28

[tqh]

Hmm, I can't see what's wrong.
The toolkit/xre file is rewritten with only nsApprunner code in it, so I didn't
bother to move over the author from xpfe/bootstrap if you meant that.

Sergei, yes I meant IRC.

Comment 29

[Niels Reedijk]

Tqh, I guess you should check the legal status of the nsNativeAppSupportBeOS.cpp
file. As it wasn't in the original Netscape suite, it should have a different
license header. I don't know which one, you can contact the Mozilla Drivers for
that.

Comment 30

[Niels Reedijk]

Attachment: Updated license header

Comment 31

[Niels Reedijk]

Attachment: Updated license header. nsNativeAppSupportBeOS.cpp for toolkit/xre

Update license header

Comment 32

[tqh]

ah, good.

Comment 33

[tqh]

Comment on attachment 178257 [details]
Updated license header. nsNativeAppSupportBeOS.cpp for toolkit/xre

Changing reviewer, r?[e-mail]

Note, This is a BeOS-specific file, so I don't expect a code review, but
approval of CVS-header and that file can be added to CVS.

Comment 34

[Niels Reedijk]

Comment on attachment 174102 [details] [diff] [review]
Same for toolkit/xre

Requesting approval for all patches (naturally).

Comment 35

[Asa Dotzler [:asa]]

Comment on attachment 174102 [details] [diff] [review]
Same for toolkit/xre

a=asa

Comment 36

[Asa Dotzler [:asa]]

Comment on attachment 175032 [details] [diff] [review]
Tried to clean up formatting

a=asa

Comment 37

[Asa Dotzler [:asa]]

Comment on attachment 178257 [details]
Updated license header. nsNativeAppSupportBeOS.cpp for toolkit/xre

a=asa

Comment 38

[Niels Reedijk]

Checked in by timeless

Comment 39

[tqh]

Excellent. Thanks Niels for getting the last hurdles out of the way. And
timeless for having time for the BeOS-port..

Comment 40

[Asa Dotzler [:asa]]

Comment on attachment 175032 [details] [diff] [review]
Tried to clean up formatting

a=asa