Native menus: events - status

Pavel Janík wrote:
> Hi,
> 
> this patch
> 
> http
://ftp.linux.cz/pub/localization/OpenOffice.org/devel/build/
Patches/SRC680/aquavcl01-complete-menus.diff
> 
> contains all my work so far in one patch, this time
with implemented menu
> item level events SALEVENT_MENUHIGHLIGHT,
SALEVENT_MENUCOMMAND. I patched
> svdem in vcl a bit (in one of the next rounds, I'll
probably remove all
> code from it and will introduce separate svmenudem.cxx
or so). We now
> support SetSelectHdl and SetHighlightHdl hopefully
properly.

I finally had a look at your patch and would like to give a
few comments:

- 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'
- AquaSalFrame::SetMenu() should check that it is a menubar
and not an 
ordinary menu that is passed, see the Windows implementation
- I think you're checking already that the userdata
parameter in menu 
events is useful to avoid crashes ?

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.
What was the problem with SetMenuItemRefCon ? Did it not
compile in 
salframe.cxx or did it just not work ?

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. Forget what I suggested above to
use 0 for it 
all the time. The Windows implementation does this in the
destructor so 
please do it there as well (ie. ~AquaSalMenu)

AquaSalMenu::InsertItem()
Again, why did SetMenuItemRefCon not work ?


void AquaSalMenu::RemoveItem
Please implement  Just to
avoid resource leaks.



Would be great to remove the '~' (just have a look at the
Windows 
implementation).
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). 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).

Anyway, we will only see if everything in the native menus
works once we 
have one of the applications running. The native menus are
only an 
add-on and VCL's internal menus do everything correctly. I
took me a 
while to get everything right for the windows menus and it
definitely 
means debugging and comparing what the VCL menus do
differently. Even 
subtle differences may lead to things not working correctly.
But this 
will not be apparent in svdem.


> I still have some questions about menu-level events
SALEVENT_MENUACTIVATE
> and SALEVENT_MENUDEACTIVATE. I still do not know, what
should I fill into
> SalMenuEvent structure. Can you help here? It is easy
in case of menu item
> level events - menu in which it happened (into mpMenu)
and the id of the
> menu item in question (mnId field). But what should be
filled there in case
> the menu "File" gets activate, e.g.?
> 
> The other question is connected with the above one.
Should I call
> SetActivateHdl and SetDeactivateHdl of *the menubar*
(aMainWin.aMenuBar in
> case of our svdem.cxx code) or of top-level menus
(aMainWin.aFileMenu)?
> 

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.

> BTW - I was also not yet able to pass the back
reference to SalMenu to
> native menus via properties, I had to use RefCons -
they are 32bit only
> though, so I hope next MB Pro with Merom chip won't
run in 64bit
> natively  For more
details, search for "SetMenuItemProperty" in the
> patch. I put comment there.

Yes I saw it but did not exactly understand what you meant.

> 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.

Stephan

------------------------------------------------------------
---------
To unsubscribe, e-mail: mac-unsubscribeporting.openoffice.org
For additional commands, e-mail: mac-helpporting.openoffice.org