Bug 326479

Comment 1 Prakash Rangaraj 2010-10-05 01:18:46 EDT

Scott,

    Some code sample/pointers would help me to use this API in Platform UI.

Comment 2 Scott Kovatch 2010-10-05 12:12:15 EDT

Sorry about that. I got wrapped up in the native toolbar work.

The idea is that once you use getAppMenuBar() you always use it. All subsequent calls to Shell.setMenuBar() are ignored after you call getAppMenuBar() once. As you found, calling it removes any existing menu bar that may have been set previously, though it doesn't dispose of the menus.

A snippet is attached; I'm going to add this as a standard snippet shortly.

Comment 3 Scott Kovatch 2010-10-05 12:14:21 EDT

Created attachment 180255 [details]
Snippet for getAppMenuBar()

This app should behave properly on Mac and Windows, though I should check it again on Windows to be sure.

Comment 4 Scott Kovatch 2010-10-05 12:17:38 EDT

Created attachment 180256 [details]
Modified AddressBook example

Modififed AddressBook example that uses menuShown events to enable and disable menu items based on the current application state.

Comment 5 Prakash Rangaraj 2010-10-05 12:47:37 EDT

(In reply to comment #2)
> The idea is that once you use getAppMenuBar() you always use it. All subsequent
> calls to Shell.setMenuBar() are ignored after you call getAppMenuBar() once. As
> you found, calling it removes any existing menu bar that may have been set
> previously, though it doesn't dispose of the menus.

    So, if Eclipse starts using the getAppMenuBar() and all workbench windows use this API. What happens to a plugin which creates a Shell & creates a menu on that shell?

Comment 6 Scott Kovatch 2010-10-05 13:04:38 EDT

(In reply to comment #5)

>     So, if Eclipse starts using the getAppMenuBar() and all workbench windows
> use this API. What happens to a plugin which creates a Shell & creates a menu
> on that shell?

I think you mean a menu bar, right? I do see the problem, though. On Cocoa that menu bar would be ignored when the window is activated, and the application menu bar would remain. A plugin would have to be rewritten to be aware of getAppMenuBar and do the right thing.

Let me  think about this a bit. The point of adding this API was to keep the menu bar from being torn down and reconstructed every time a Shell activates or deactivates, but I see there might be backwards compatibility issues to deal with.

Comment 7 Scott Kovatch 2010-10-05 13:48:54 EDT

Created attachment 180262 [details]
Snippet for getAppMenuBar()

Updated snippet that works on Windows too.

Comment 8 Scott Kovatch 2010-10-05 13:56:01 EDT

Created attachment 180265 [details]
Fix

This patch will allow a shell to override the screen menu bar while it is active. If a shell does not have its own menu bar the app menu bar will be used instead. If the application is only using the app menu bar there will be no performance hit as the menu bar won't change each time a window changes activation.

Comment 9 Scott Kovatch 2010-10-05 14:04:36 EDT

Created attachment 180266 [details]
Better fix

This fix doesn't clear the menu bar when getAppMenuBar() is called and another menu bar is already in place. The app menu bar will be used when the next Shell without a menu bar is activated or when no Shell is active.

Comment 10 Scott Kovatch 2010-10-05 14:18:59 EDT

Created attachment 180271 [details]
Modified version of test case

Slightly modified version of your test case that doesn't exit when the shell closes. Remember to click the button, but note that the app menu bar shows when the shell closes.

Comment 11 Scott Kovatch 2010-10-05 15:55:24 EDT

Fixed > 20101005. If there is a menu bar already in place, getAppMenuBar won't clear it. Also, if a shell calls setMenuBar, it is used when the shell activates. If a shell does not have a menu bar the app menu bar is used, if it was created. If code is just using the app menu bar, the menu bar will not be cleared and reinstalled each time a shell activates.

Comment 12 Prakash Rangaraj 2010-10-08 01:26:29 EDT

Scott,

   The appMenu is not firing Menu listeners. Is it supposed to be that way? Or should I open a new bug for that?

Comment 13 Scott Kovatch 2010-10-08 03:08:49 EDT

(In reply to comment #12)
> Scott,
> 
>    The appMenu is not firing Menu listeners. Is it supposed to be that way? Or
> should I open a new bug for that?

Oops. No, that's not right. Please file a new bug; it should be pretty simple to fix.

Comment 14 alf 2010-10-31 23:41:26 EDT

Hi! 
I want to implement appmenu to RCP, how can do it?
I found that MenuManager.createMenuBar() method create menu. I just change it,like this:
   public Menu createMenuBar(Decorations parent) {
        if(!menuExist()){
            -   menu = new Menu(parent, SWT.BAR);
            +   menu = parent.getDispaly().getAppMenuBar();
               update(false);
        }
        return menu;
   )

But, Running this RCP I get an Exception:
   Widget has the wrong parent come from Decorations.setMenuBar(Decorations.java 601).

Please give me a help. I think add application menu to RCP, maybe need a lot of work.

Comment 15 Prakash Rangaraj 2010-11-01 01:02:45 EDT

(In reply to comment #14)
> Hi! 
> I want to implement appmenu to RCP, how can do it?
> I found that MenuManager.createMenuBar() method create menu. I just change
> it,like this:
>    public Menu createMenuBar(Decorations parent) {
>         if(!menuExist()){
>             -   menu = new Menu(parent, SWT.BAR);
>             +   menu = parent.getDispaly().getAppMenuBar();
>                update(false);
>         }
>         return menu;
>    )
> 
> But, Running this RCP I get an Exception:
>    Widget has the wrong parent come from
> Decorations.setMenuBar(Decorations.java 601).
> 
> Please give me a help. I think add application menu to RCP, maybe need a lot of
> work.

You can't use the appmenu on the shell like shell.setMenu(appMenu). Each shell will have a menu bar (created with Shell as the parent) + one more menu bar for the entire application. I've got an example snippet here: http://blog.eclipse-tips.com/2010/10/new-features-on-mac.html

There is more work being done on supporting this on an RCP level. Please cc yourself to Bug#74073 for updates