Bug 348421

Comment 1 Chris Goldthorpe 2011-06-06 13:02:16 EDT

Do you have a suggested patch which would implement this?

Comment 2 Chris Memory 2011-06-07 02:17:13 EDT

(In reply to comment #1)
> Do you have a suggested patch which would implement this?

Not exactly... there are two possible solutions, both with potential downsides I cannot figure out for sure.


1) MenuManager simply gets a setter for its id:     

   // MenuManager:
   public void setId(String id) {
      this.id = id;
   }


...this might be a problem if messing up caching mechanisms in the Platform I am not aware of.



2) Form/FormHeading/TitleRegion each gets a ID setter that is used at the lazy init, which of course is really a cumbersome and unelegant solution:

   // a) Form:
   public void setMenuId(String id) {
      head.setMenuId(id);
   }

   // b) FormHeading:
   public void setMenuId(String id) {
      titleRegion.setMenuId(id);
   }

   // c) TitleRegion:
   public void setMenuId(String id) {
      this.menuId = id;
   }

   public IMenuManager getMenuManager() {
      if (menuManager == null) {
         menuManager = new MenuManager(this.menuId); // <-- use ID here
         Menu menu = menuManager.createContextMenu(this);
         setMenu(menu);
         titleLabel.setMenu(menu);
         if (busyLabel != null)
            busyLabel.setMenu(menu);
         createMenuHyperlink();
      }
      return menuManager;
   }
   

I would prefer solution 1)... However, if there actually IS a problem with ID's that change suddenly, maybe there might be an assertion that the id is only written once (right after creation)?

1*) MenuManager 

   private boolean idWasRead = false;
   
   public void setId(String id) {
      if (this.id != null || idWasRead ) {
         throw new IllegalStateException(
                "MenuManager's ID may only be set once after creation!");
      }
      this.id = id;
   }

   public void getId() {
      idWasRead = true;
      return id;
   }

Comment 3 Paul Webster 2011-06-07 08:03:56 EDT

Just FYI:  the menu manager id can only be set once on creation.  We can't vary this now.

I'll also mention, did you know you can use the IMenuService menu contribution mechanism to contribute to a MenuManager you control.  ex:

IMenuService srv = (IMenuService) getSite().getService(IMenuService.class);
MenuManager manager = form.getMenuManager();
srv.populateContributionManager(manager, "menu:form.menu.id1");

When you dispose the part and/or form, you simply go:

IMenuService srv = (IMenuService) getSite().getService(IMenuService.class);
srv.releaseContributions(manager);

Then any contributions to org.eclipse.ui.menus with the different locationURIs will get populated.

PW

Comment 4 Chris Memory 2011-06-08 03:06:03 EDT

(In reply to comment #3)
> Just FYI:  the menu manager id can only be set once on creation.  We can't vary
> this now.

I expected that and I can follow You on that :)

 
> I'll also mention, did you know you can use the IMenuService menu contribution
> mechanism to contribute to a MenuManager you control.  ex:
> PW

Yes, thank you. However, I currently try to figure out if one of my popup-menus will have menu items (added through whatever mechanism the platform supports) _without_ showing it in the firsthand. 
Unfortunately, the handling PopupMenuExtender.menuAboutToShow(MenuManager) does a lot more than the IMenuService alone so I fear to miss non-contributed menu-items.

About the MenuManager.getId(): I was not aware and am a little puzzled that the menu-id may be specified also via *Site.registerContextMenu(...) which seems some kind of a redundancy to the id held in the MenuManager. (I interpreted by that Method MM.getId() somehow and expected the registerContextMenu always uses this id of the MM - that explains why I try to set it)

This confusing fact sounds like someone already figured out some kind of a work-around the same problem (MM.getId() not alterable). Asked the other way around: isn't the id of the MM thus a little useless and obsolete and could be removed or deprecated? Then at least no others users would be lead into the wrong direction :-)

Comment 5 Paul Webster 2011-06-08 07:36:42 EDT

(In reply to comment #4)

> About the MenuManager.getId(): I was not aware and am a little puzzled that the
> menu-id may be specified also via *Site.registerContextMenu(...) which seems
> some kind of a redundancy to the id held in the MenuManager.

I didn't realize you were using registerContextMenu(*).  The problem is they're not used for the same thing.

The ID for a MenuManager is used to identify that MM.  It's used by the framework when feeding it into the IMenuService, and it's used by all part of the Main Menu (File has an ID of "file", etc).  In all of those cases, it's ID is immutable.

> This confusing fact sounds like someone already figured out some kind of a
> work-around the same problem (MM.getId() not alterable). Asked the other way
> around: isn't the id of the MM thus a little useless and obsolete and could be
> removed or deprecated? Then at least no others users would be lead into the
> wrong direction :-)

With registerContextMenu(*) it's actually keeping track of a set of IDs, that are all applied to the context menu on menuAboutToShow(*) and have no relation to the root context MenuManager.  ex: The Java editor context menu picks up 4 popup IDs, 2 from TextEditor ('cause its a text editor) and 2 from JavaEditor.

I don't understand what you are trying to do with the popup menu ... as the common case will remove all items and re-fill it on every menuAboutToShow(*), based on the selection provider (viewer, usually) it was registered with.  This only applies to menus run through registerContextMenu(*), however.

PW

Comment 6 Chris Memory 2011-06-10 03:28:19 EDT

(In reply to comment #5)

> I didn't realize you were using registerContextMenu(*).  The problem is 
> they're not used for the same thing.

Yes, I start to figure that out. Maybe I missed the whole point of the MM id... of course it's correctly named "id", but I (miss?) interpreted it as "menu-ID". I guess that's what I got all wrong.

> I don't understand what you are trying to do with the popup menu ... as the
> common case will remove all items and re-fill it on every menuAboutToShow(*),
> based on the selection provider (viewer, usually) it was registered with.  This
> only applies to menus run through registerContextMenu(*), however.

Thanx for Your replies. My intention in the first place was to check if a menu, that was not yet shown (i.e. was not yet lazy filled up), would have _some_ entries -- MenuManager.isEmpty() does only work if the menu showed at least once.

Short version: i tried to fill up a MM's menu without actually showing it... it does not seem to be possible, so I guess I will try to create another request for that matter.... but not before I did some further investigations ;-)

I think my request becomes obsolete now.

Comment 7 Lars Vogel 2019-11-14 03:14:51 EST

This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet.

If you have further information on the current state of the bug, please add it. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant.

If the bug is still relevant, please remove the "stalebug" whiteboard tag.