From: Stephan Schaefer <s.schaefer sun.com>
Date: Fri, 28 Jul 2006 12:41:08 +0200
Stephan,
thank you for your exhaustive reply I'll try
to comment everything you
wrote inline:
> - please start all class member names with a 'm'
and try to indicate
> what Type it is with the second letter. So pointers
should start with
> 'mp' (eg, do not use myMenu but mpMenu),
references would use 'mr',
> inegers/counters 'mn', booleans 'mb' etc. For
instances of other classes
> or types use 'ma'
I hope I have changed the naming properly in the new patch:
ftp://ftp.linux.cz/pub/localization/OpenOffice.org/devel/bui
ld/Patches/SRC680/aquavcl01-complete-menus-20060728.diff
One question about references - you meant references
probably, but I have
chosen it use mr prefix also for MenuRefs. Is it OK or
should I rename
them? Really no problem, just say so please.
> - AquaSalFrame::SetMenu() should check that it is a
menubar and not an
> ordinary menu that is passed, see the Windows
implementation
I have investigated it a bit and in Windows menu
implementation, you have
to distinguish between Menu and PopupMenu so you can call
the proper
creation function and probably can't call ::SetMenu for
PopupMenu or it
will fail or whatever (I do not know the API there, of
course).
In Carbon, both menu bar and menu are MenuRefs, and the only
difference is
our BOOL mbMenuBar. So it is a question, if we want to add
additional check there
(in the sense: "user" wants to do it, so why
not? . I do
not know - what
do you think? It is mainly VCL question, not Mac In fact
it is one
liner anyway
> - I think you're checking already that the userdata
parameter in menu
> events is useful to avoid crashes ?
Not yet unfortunately - GetMenuItemRefCon's result it not
yet properly
checked in HandleMenuPopulateEvent and HandleMenuClosedEvent
- I'll try to
look at it today before our IRC meeting.
> AquaSalInstance::CreateMenu()
> no idea if the menuid is required, we're not using
it right ? did you
> try to set it to 0 all the time, but this might not
work for Carbon if
> all menus have the same id. Otherwise it's fine to
use the static counter.
MenuID: yes, looks like we do not need it at all - I thought
it could be
useful later. But as I look closely on it, we do not need it
at all - we
store MenuRef for every menu and there is a function
GetMenuID (http://developer.apple.com/documentation/Carbon/
Reference/Menu_Manager/Reference/reference.html#//apple_ref/
c/func/GetMenuID)
GetMenuID (Obtains the ID of a menu):
MenuID GetMenuID (
MenuRef menu
);
So we do not need it at all -> I'll use static counter
for menuIDs but will
not store them, OK? BTW - is it OK to have the static
variable there? Or
should I move it "up" to be a member of
AquaSalInstance instead? What is
more clean?
Current patch for this:
ftp://ftp.linux.cz/pub/localization/OpenOffice.org/devel/bui
ld/Patches/SRC680/aquavcl01-01-do-not-store-MenuIDs.diff
> What was the problem with SetMenuItemRefCon ? Did it
not compile in
> salframe.cxx or did it just not work ?
I'll try to provide patch demonstrating my problems later
(I still have
this on my TODO). The problem was that I have used
GetMenuItemProperty and
SetMenuItemProperty first. In the function, I set the
property using the
same value for it as I do not with RefCons, I was able to
again Get it
immediately, but when I run the same Get function on the
same object in
salframe, I have received NULL instead. I'll investigate
more and will
provide demo patch that is using both these methods to pass
the back
reference and we will see
> AquaSalInstance: estroyMe
nu()
> you must delete the Carbon menu as well, it seems
that DeleteMenu is the
> appropriate call - and this is where you need the
menu id so you need
> individual values for it.
I do not think so - of course I have to delete the menu, but
DeleteMenu
seems to be used for removing it from the *current* menu
list. I think
I should use DisposeMenu instead:
http://developer.apple.com/documentation/Carbo
n/Reference/Menu_Manager/Reference/reference.html#//apple_re
f/c/func/DisposeMenu
> all the time. The Windows implementation does this
in the destructor so
> please do it there as well (ie. ~AquaSalMenu)
Will do - thanks for the link to the source.
> AquaSalMenu::InsertItem()
> Again, why did SetMenuItemRefCon not work ?
The same problem as above. RefCons work, properties do not.
I was not able
to Get the property value I set elsewhere.
> void AquaSalMenu::RemoveItem
> Please implement Just to
avoid resource leaks.
Will do
> Would be great to remove the '~' (just have a look
at the Windows
> implementation).
OK, OK. Consider it done This was
so low prio me, that I completely
forgot it
Patch for this is at
ftp://ftp.linux.cz/pub/localization/OpenOffice.org/devel/bui
ld/Patches/SRC680/aquavcl01-02-remove-tildas-use-XubString.d
iff
I decided to change also the type of mText back to XubString
as used by
Windows implementation. I haven't yet checked what
"~" is doing in the
top-level menu titles though (IIRC they are set using the
mText of the
first item in the menu anyway).
> Regarding the activate events, I think you already
got an answer from
> pl. The most important thing is to keep activate and
deactivate always
> in sync, there must be no more activates than
deactivates and vice
> versa. In the Windows implementation I send a
deactivate immediately
> after an activate which works pretty well
(salframe.cxx,
> ImplHandleMenuActivate).
But you do not draw the menus on Windows. I think we should
not send the
deactivate immediatelly after the activate event, because we
actually
display the menus and our "user" can potentially
really expect the menu is
NOT shown when he received DEACTIVATE event. Or no?
> You must send those for the menubar (i.e. once for
each frame, I keep
> track of this on Windows with the
mLastActivatedhMenu member) and for
> each opening of a popupmenu (i.e. multiple times).
You that I have to send activate event in these cases:
- (can send) menubar is selected the first time
- top-level menu is about ot be displayed
What you mean with "(once for each frame)"? Can
we speak in terms of
menubar with menus and menu items in the current menu patch?
Maybe this
will be clear when we have more windows/frames in svdem
Do you mean I should/could send ACTIVATE event to the
menubar when the user
enters menu tracking session and DEACTIVATE when he finishes
menu tracking
session? And if he is about to display File menu, I should
send ACTIVATE to
the menu itself and the same for DEACTIVATE? I think I'm
getting closer to
it.
I'm probably fooled by the fact, that you can not activate
the application
menu bar without activating one of the top-level menus Activating
the
menubar and "about to display top-level menu"
are two distinct events!
Right? If so, I'm a bit slow
> Anyway, we will only see if everything in the native
menus works once we
> have one of the applications running.
Yes, of course - I'd be more than happy to see something
running - maybe
testtool is the simplest to get it runnign? Eric wants to
see an app named
soffice.bin
> You probably won't need it at all. Unless you
wanted to execute code
> during menu activation. The applications to it for
the menubar and for
> the menus, as they require it for the program logic.
Yes - I even extended svdem to see them, but handlers I set
were not
called. But I still have to inevstigate this.
I already debug-print where I'm about to send these events:
+ fprintf(stderr, "PJ: MenuClosed,
SALEVENT_MENUDEACTIVATE event!\n");
But the code below was not working properly - but I probably
understand it
now - I installed the handlers into menubar and was sending
them for menus
or vice-versa! Probably. Will try it later.
> > I'll now slow down a bit for a few days and
hope to do something more next
> > week and receive your comments
>
> Sorry that it took so long.
No problem - thank you very much for your time. Without your
document about
native menus and without your suggestions, I'm still
studying VCL's
inc/*menu* files! Thank you!
--
Pavel Janík
I wouldn't like to have something like fetchmail for the
kernel
configuration.
-- Martin Dalecki in LKML about CML2
------------------------------------------------------------
---------
To unsubscribe, e-mail: mac-unsubscribe porting.openoffice.org
For additional commands, e-mail: mac-help porting.openoffice.org
|