Bugzilla – Full Text Bug Listing
| Summary: | [launch][menu] LaunchingResourceManager's mouse listener will not be attached in 4.x | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Remy Suen <remy.suen> |
| Component: | UI | Assignee: | Paul Webster <pwebster> |
| Status: | VERIFIED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P3 | CC: | daniel_megert, emoffatt, Michael_Rennie, pawel.1.piech, pwebster |
| Version: | 4.2 | ||
| Target Milestone: | 4.2 M7 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
|
Description
Remy Suen
What was this used for ? (In reply to comment #1) > We no longer use CoolBars in 4.x the code in LaunchingResourceManager's > addMouseListener(IWorkbenchWindow) method won't actually get called. What should we be using instead? How do you show the debug toolbar in 4.x? > What was this used for ? The mouse listener allows the context launching framework to compute the label for the run / debug button when you mouse over it. (In reply to comment #2) > What should we be using instead? Not quite sure yet at the moment. > How do you show the debug toolbar in 4.x? There are no more SWT custom CoolBars. Just regular native ToolBar widgets. So the actual problem is that you want to delay computing the button labels until the user is close (otherwise contextual launch will force changes frequently)? I'm trying to think of a more API-friendly approach that would work in 3.x and 4.x, but in the mean time what if you change: CoolBarManager cmgr = ((WorkbenchWindow)window).getCoolBarManager(); to ICoolBarManager cmgr = ((WorkbenchWindow)window).getCoolBarManager2(); in 3.7. I'm probably going to be providing an implementation of ICoolBarManager (backed by the model) as part of my efforts to fix editor toolbars. PW (In reply to comment #4) > So the actual problem is that you want to delay computing the button labels > until the user is close (otherwise contextual launch will force changes > frequently)? > yes, we want to be able to update the button hover text with a meaningful representation of what context launching is proposing to do, but *not* until they will care about the label (hovering over the button). > I'm trying to think of a more API-friendly approach that would work in 3.x and > 4.x, but in the mean time what if you change: > CoolBarManager cmgr = ((WorkbenchWindow)window).getCoolBarManager(); > > to > ICoolBarManager cmgr = ((WorkbenchWindow)window).getCoolBarManager2(); > in 3.7. Sure, we can try it. Committed Paul's suggestion to HEAD. I'll leave the bug open for discussion about what is needed moving forward. Why is this a bug in Platform Debug? As far as I can see Debug uses API and plug-ins using API should work on 4.2 without any breakage. (In reply to comment #7) > Why is this a bug in Platform Debug? As far as I can see Debug uses API and > plug-ins using API should work on 4.2 without any breakage. Note that this is more targeted at comment 0. The following Debug code: ICoolBarManager cmgr = ((WorkbenchWindow)window).getCoolBarManager2(); looks like internal access but getCoolBarManager2() is implemented in ApplicationWindow which is public API, hence Debug could do all of this by only using APIs. (In reply to comment #7) > Why is this a bug in Platform Debug? As far as I can see Debug uses API and > plug-ins using API should work on 4.2 without any breakage. I've confirmed that the action method is being called so I'm moving back. (In reply to comment #8) > The following Debug code: > ICoolBarManager cmgr = ((WorkbenchWindow)window).getCoolBarManager2(); > looks like internal access but getCoolBarManager2() is implemented in > ApplicationWindow which is public API, hence Debug could do all of this by only > using APIs. While this statement is true (not technically reaching into intenals), this code path would only work if you assume an IWorkbenchWindow is backed by an ApplicationWindow, which it isn't in 4.x. Though yes, we do have a getCoolBarManager2() method in 4.x's WorkbenchWindow but WorkbenchWindow does not extend ApplicationWindow in 4.x. (In reply to comment #9) > (In reply to comment #7) > > Why is this a bug in Platform Debug? As far as I can see Debug uses API and > > plug-ins using API should work on 4.2 without any breakage. > > I've confirmed that the action method is being called so I'm moving back. Good, thanks. > but WorkbenchWindow does not extend ApplicationWindow in 4.x. Right and that is an API breakage. (In reply to comment #10) > (In reply to comment #9) > > (In reply to comment #7) > > > Why is this a bug in Platform Debug? As far as I can see Debug uses API and > > > plug-ins using API should work on 4.2 without any breakage. > > > > I've confirmed that the action method is being called so I'm moving back. > Good, thanks. > > > but WorkbenchWindow does not extend ApplicationWindow in 4.x. > > Right and that is an API breakage. See http://wiki.eclipse.org/Evolving_Java-based_APIs_2, Contract superclass set (direct or inherited) (In reply to comment #11) > > > but WorkbenchWindow does not extend ApplicationWindow in 4.x. > > > > Right and that is an API breakage. > See http://wiki.eclipse.org/Evolving_Java-based_APIs_2, Contract superclass set > (direct or inherited) I think I'm missing something here, Dani. WorkbenchWindow is an internal class in the org.eclipse.ui.internal package. (In reply to comment #11) > > > but WorkbenchWindow does not extend ApplicationWindow in 4.x. > > > > Right and that is an API breakage. > See http://wiki.eclipse.org/Evolving_Java-based_APIs_2, Contract superclass set > (direct or inherited) WorkbenchWindow was never API. We only ever returned IWorkbenchWindow. There has never been any API way to access the ApplicationWindow for things like the menu bar MenuManager or the CoolBarManager. PW (In reply to comment #13) > (In reply to comment #11) > > > > but WorkbenchWindow does not extend ApplicationWindow in 4.x. > > > > > > Right and that is an API breakage. > > See http://wiki.eclipse.org/Evolving_Java-based_APIs_2, Contract superclass set > > (direct or inherited) > > WorkbenchWindow was never API. We only ever returned IWorkbenchWindow. There > has never been any API way to access the ApplicationWindow for things like the > menu bar MenuManager or the CoolBarManager. Right, but I could do an instanceof ApplicationWindow and then do the cast + call. As outline in (4) of the API Guidelines: (4) Shrinking an API class's set of API superclasses and superinterfaces (either directly or inherited) breaks compatibility because some casts in pre-existing Client code will no longer work. I agree that what we have here is an edge case. (In reply to comment #14) > > Right, but I could do an instanceof ApplicationWindow and then do the cast + > call. As outline in (4) of the API Guidelines: > > (4) Shrinking an API class's set of API superclasses and superinterfaces > (either directly or inherited) breaks compatibility because some casts in > pre-existing Client code will no longer work. > That doesn't apply here, as I read it. If they had access to WorkbenchWindow as API and we removed some parent classes like ApplicationWindow, I can see that statement would apply (because it would shrink the API class' superclass). But consumers only had access to an instance object, not a class, and we only promised the instance would implement IWorkbenchWindow. PW (In reply to comment #15) > (In reply to comment #14) > > > > Right, but I could do an instanceof ApplicationWindow and then do the cast + > > call. As outline in (4) of the API Guidelines: > > > > (4) Shrinking an API class's set of API superclasses and superinterfaces > > (either directly or inherited) breaks compatibility because some casts in > > pre-existing Client code will no longer work. > > > > That doesn't apply here, as I read it. If they had access to WorkbenchWindow > as API and we removed some parent classes like ApplicationWindow, I can see > that statement would apply (because it would shrink the API class' superclass). > But consumers only had access to an instance object, not a class, and we only > promised the instance would implement IWorkbenchWindow. > I agree with you in general Paul. As said, it's a corner case. Fact is, that pure API based client code fails (experiences a semantic change). So, assuming we won't restore the hierarchy: what does Debug need to do to get the menu working again? (In reply to comment #16) > So, assuming we won't restore the hierarchy: what does Debug need to do to get > the menu working again? While debugging bug 317182 I found that the toolbar actions do not get called to initialize until you use them. For example we relied on a call-back to org.eclipse.ui.IActionDelegate2#init(org.eclipse.jface.action.IAction) to attach our label listeners, in 3.x this worked fine - as soon as the window was shown the action initialized et viola!. In 4.x you have to click the button for the init call-back to happen. Pawel / Remy / Eric, is there a new way to be notified when the tool bar action get initialized? Is this the intended behavior of toolbar items? The next part of the problem is that the callback into CommandActionLegacyWrapper#setToolTipText tries to set the description of the action... > Pawel / Remy / Eric, is there a new way to be notified when the tool bar action
> get initialized?
As long as APIs were used, it doesn't matter whether there's a new way - it should just work.
(In reply to comment #18) > While debugging bug 317182 I found that the toolbar actions do not get called > to initialize until you use them. For example we relied on a call-back to > org.eclipse.ui.IActionDelegate2#init(org.eclipse.jface.action.IAction) to > attach our label listeners, in 3.x this worked fine - as soon as the window was > shown the action initialized et viola!. In 4.x you have to click the button for > the init call-back to happen. Apparently 4.x does a better job of lazy loading than 3.x. > > The next part of the problem is that the callback into > CommandActionLegacyWrapper#setToolTipText tries to set the description of the > action... This we have to hook up correctly, so that it can update the tooltip on the toolitem created by the renderers. PW using Version: 4.2.0 Build id: I20120321-0610 and the latest code from master, the debug tooltips are updating. I think we can close this bug and track any other issues with the correctness of the tooltip or tooltip setting in bug 317182 Marking as fixed, since the changes from Paul's legacyActions branch changed how the callback to set a tooltip is handled verified in Version: 4.2.0 Build id: I20120430-1800 |