58217 - Fix to nsWindows - get menus,popups,dialogs,tooltips to work

Status: VERIFIED FIXED

(Core :: XUL, defect, P3)

Description

[Makoto Hamanaka]

Until now, menus, popups, dialogs, tooltips didn't appear on BeOS port.
That is because nsWindow creates only BView when it needed to create a new 
window(BWindow).
I made a patch to fix it.

Comment 1

[Makoto Hamanaka]

Attachment: patch to widget/src/beos/nsWindow.cpp , widget/src/beos/nsWindow.h

Comment 2

[Blake Ross]

cc'ing trudelle and danm to do something here so this patch doesn't rot away...

Comment 3

[Peter Trudelle]

cc mcafee.  Who owns the BeOS code?  

Comment 4

[Yannick Koehler]

I have received comments that this patch is wrongly done.  I had computer problems and was unable to evaluate or commit it.  I am back on track now with a new motherboard and BeOS seems more stable on it now.  I will check this patch tomorrow and if ok will get to the review process.

Comment 5

[Yannick Koehler]

We need someone to review this patch from the widget module.

Comment 6

[timeless]

Attachment: cvs diff -u, -uw. Includes code cleanup

Comment 7

[timeless]

style wise, I think we prefer the following:
PRBool > bool
return x > return (x)
nsnull > NULL||null
if (a) > if (a!=nsnull)
if (!a) > if (a==nsnull)
q a(b) > q a=b;
A::A():c(q){ > A::A(){c=q;
Follow Indentline and don't use tabs.
Try to limit lines to 80 chars.

Comment 8

[Dan M]

Yannick, Makoto: what do you guys need here? I'd like you to get the reviewed by 
someone who does know Be (looks like you are...), but if you need some windowing 
code co-owner type-person rubber-stampage to get it checked in, I'll OK it. I 
don't know Be, so I can't really say anything useful about your code's logic. But 
I've glanced through it, and it seems like a good thing, pending a real review. 
Other platforms are unaffected. Mostly new functionality. Go for it.

Comment 9

[Yannick Koehler]

Yeah I need an approval stamp.  The review that need to be approved is the 
scope, you know, ensure that It won't break other platform.

I have reviewed that patch, we could do better but for now I think this will 
do. Just for ... the procedure can you put the r=, I've talked with Blizzard 
and as long as I got a reviewer from the module owner or peer I can go ahead 
without an sr= for beos code only.

Comment 10

[Dan M]

r=danm

Comment 11

[Yannick Koehler]

I had to modify patch 20335 has the initialization of the nsWindow was using the variable "back" which was declared inside the function (which is after it's used..., wonder how someone did get that compiled...)

I tried the first patch 18144, while the second one seems to respect more the coding guide of mozilla it doesn't work properly.  And also change more stuff than the first patch did.

Therefore I checked in the 18144.  In a future patch we will correct all the coding guidelines stuff in one shot.  As this only affect BeOS code.  

I've checked that the menu now works and they appear.  I've played a little on it and seems quite solid, got a crash but unrelated.

Comment 12

[Yannick Koehler]

commit done.  Now watching tinderbox.

Comment 13

[Makoto Hamanaka]

Thank you, Yannick.
I tested and it seems good.

Comment 14

[Yannick Koehler]

Let me know the next patch, also ensure we get review.

Comment 15

[Kai Lahmann (is there, where MNG is)]

v