Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 346543

Summary: [Contributions] [JFace] MenuManager should identify its widgets
Product: [Eclipse Project] Platform Reporter: Paul Webster <pwebster>
Component: UIAssignee: Paul Webster <pwebster>
Status: VERIFIED FIXED QA Contact: Paul Webster <pwebster>
Severity: normal    
Priority: P3 CC: bokowski, daniel_megert, emoffatt, ob1.eclipse, prakash, remy.suen
Version: 3.7Flags: remy.suen: review+
emoffatt: review+
bokowski: review+
Target Milestone: 3.7 RC3   
Hardware: All   
OS: All   
Whiteboard:
Bug Depends on:    
Bug Blocks: 332246    
Attachments:
Description Flags
MenuManager is data on its widget none

Description Paul Webster CLA 2011-05-19 17:06:16 EDT
The MenuManager should store itself in the menu widget it creates.  This follows the pattern used my the other ContributionItems, and will allow the manager to be found from its widget.

PW
Comment 1 Paul Webster CLA 2011-05-19 17:11:30 EDT
Created attachment 196170 [details]
MenuManager is data on its widget

Store the MenuManager as data on its widget.

PW
Comment 2 Paul Webster CLA 2011-05-19 17:12:16 EDT
I need 2 commiters to review, and Boris, I need approval for you for this change.

PW
Comment 3 Paul Webster CLA 2011-05-19 17:12:38 EDT
Eric, could you please review?

PW
Comment 4 Boris Bokowski CLA 2011-05-19 20:47:31 EDT
Is this needed to support the Eclipse 4.x compatibility story?
Comment 5 Paul Webster CLA 2011-05-19 22:07:04 EDT
(In reply to comment #4)
> Is this needed to support the Eclipse 4.x compatibility story?

Yes, bug 332246 (a P1).  I've explored many options and this is the only way to get from the SWT Menu to something useful that first time.

PW
Comment 6 Boris Bokowski CLA 2011-05-20 08:45:21 EDT
(In reply to comment #5)
> > Is this needed to support the Eclipse 4.x compatibility story?
> 
> Yes, bug 332246 (a P1).  I've explored many options and this is the only way to
> get from the SWT Menu to something useful that first time.

+1 from me.
Comment 7 Remy Suen CLA 2011-05-20 08:53:55 EDT
I am okay with this on the condition that the string be changed to be something like "org.eclipse.ui.internal.managerKey" to prevent any potential clashing with downstream clients of getData(String) [this late in the cycle].

Patch seems innocuous enough otherwise.
Comment 8 Eric Moffatt CLA 2011-05-20 09:59:27 EDT
Looks fine to me but adopting something more like Remy's suggestion for the key string makes sense (even though I'd be willing to bet money it wouldn't be an issue as is we may as well make the chances even smaller...;-).
Comment 9 Paul Webster CLA 2011-05-20 15:47:25 EDT
Released with a longer key.
PW
Comment 10 Dani Megert CLA 2011-05-23 02:13:22 EDT
Boris, I assume you did want to set the normal +review flag.
Comment 11 Boris Bokowski CLA 2011-05-23 07:51:40 EDT
(In reply to comment #10)
> Boris, I assume you did want to set the normal +review flag.

Indeed. Thanks!
Comment 12 Paul Webster CLA 2011-05-24 10:28:58 EDT
In I20110523-0800
PW