Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 346543 - [Contributions] [JFace] MenuManager should identify its widgets
Summary: [Contributions] [JFace] MenuManager should identify its widgets
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.7   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.7 RC3   Edit
Assignee: Paul Webster CLA
QA Contact: Paul Webster CLA
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 332246
  Show dependency tree
 
Reported: 2011-05-19 17:06 EDT by Paul Webster CLA
Modified: 2011-05-24 10:28 EDT (History)
6 users (show)

See Also:
remy.suen: review+
emoffatt: review+
bokowski: review+

Attachments
MenuManager is data on its widget (1.34 KB, patch)
2011-05-19 17:11 EDT, Paul Webster CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
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