| Summary: | [KeyBindings] Keybindings incorrectly disabled and not re-enabled (keybindings lost) | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Mark Melvin <mark.melvin> | ||||
| Component: | UI | Assignee: | Paul Webster <pwebster> | ||||
| Status: | RESOLVED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | abuehler, daniel_megert | ||||
| Version: | 3.2 | ||||||
| Target Milestone: | 3.3 | ||||||
| Hardware: | PC | ||||||
| OS: | Windows XP | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
|
Description
Mark Melvin
Thanx for the tracing. I'm interested in every mention of com.amis.skt.build.ui.buildCleanCommand (and the few lines surrounding it) between: ## HERE I AM SWITCHING EDITORS TO THE *OTHER* EDITOR WHERE I EXPECT THE and ## I PRESS CTRL+SHIFT+C AND NOW IT DOESN'T WORK IN THE ORIGINAL EDITOR. GRRR. Could you attach those as well (please put your ## comments in, if you could, that helps give me context)? Thanx, PW A little more information on this bug. I have put a breakpoint on the "setEnabled" method of the specific instance of CommandLegacyActionWrapper that corresponds to my action and preventing the keybinding from executing (via CommandLegacyActionWrapper#isEnabledDisregardingCommand returning false). It is interesting because what is actually disabling the CommandLegacyActionWrapper is my action delegate, from its "selectionChanged" method. The stack trace shows that the first time the keybinding is erroneously executed in the editor in which the actions are already disabled, the ActionDelegateHandlerProxy#execute method manually fires my delegate's selectionChanged method (line 249), thus disabling the CommandLegacyActionWrapper, and hosing the keybinding. It will never be re-enabled because the ActionDelegateHandlerProxy is never executed again. It seems like this would be a nightmare to keep the enablement of the CommandLegacyActionWrapper synchronized with the enablement of the real action. I'll give you more context in a few minutes... Hey Paul, Actually, that is my entire trace. I didn't leave anything out, and all I did between the two points you mention was switch editors, and press Ctrl+Shift+C. What sort of additional info were you looking for? I guess the other key point here is that my delegate is enabling/disabling the action when the selection changes. The problem seems to be that the ActionDelegateHandlerProxy is passing its action (an instance of CommandLegacyActionWrapper) to my delegate as well and the action wrapped by the keybinding is getting disabled. I can see this by printing the instance of IAction passed to my delegate's selectionChanged method to the console (note I have three separate actions that all use a common action delegate superclass): ## change selection in the platform selectionChanged called with: org.eclipse.ui.internal.WWinPluginAction@a3cf3e selectionChanged called with: org.eclipse.ui.internal.WWinPluginAction@1635a89 selectionChanged called with: org.eclipse.ui.internal.WWinPluginAction@124527f ## change selection in the platform selectionChanged called with: org.eclipse.ui.internal.WWinPluginAction@a3cf3e selectionChanged called with: org.eclipse.ui.internal.WWinPluginAction@1635a89 selectionChanged called with: org.eclipse.ui.internal.WWinPluginAction@124527f ## Press Ctrl+Shift+C in a normal editor and a build occurs selectionChanged called with: org.eclipse.ui.internal.handlers.CommandLegacyActionWrapper@5be0a8 ## Press Ctrl+Shift+C in a bad editor and a build erroneously occurs ## (here I lose my keybindings) selectionChanged called with: org.eclipse.ui.internal.handlers.CommandLegacyActionWrapper@5be0a8 ## Press Ctrl+Shift+C again and nothing happens ## change selection in the platform selectionChanged called with: org.eclipse.ui.internal.WWinPluginAction@a3cf3e selectionChanged called with: org.eclipse.ui.internal.WWinPluginAction@1635a89 selectionChanged called with: org.eclipse.ui.internal.WWinPluginAction@124527f ## Press Ctrl+Shift+C again and nothing happens ## change selection in the platform selectionChanged called with: org.eclipse.ui.internal.WWinPluginAction@a3cf3e selectionChanged called with: org.eclipse.ui.internal.WWinPluginAction@1635a89 selectionChanged called with: org.eclipse.ui.internal.WWinPluginAction@124527f ## Press Ctrl+Shift+C again and nothing happens ..etc. So, one option would be to not enable/disable the action from my delegate if it looks like an instance of org.eclipse.ui.internal.handlers.CommandLegacyActionWrapper, but that makes me depend on an internal class, and is a hack. I'm wondering if the ActionDelegateHandlerProxy needs to call refreshEnablement() somewhere more regularly... One other odd thing I noted is that ActionDelegateHandlerProxy.selectionChanged() is not called when a selection change occurs. Should this not be called whenever a selection change occurs in the platform? If it was, I think the problem I am seeing would go away. Created attachment 42643 [details]
Patch for ActionDelegateHandlerProxy
Here is a patch that solves my problem. Not knowing enough about the command/handler infrastructure, I'm not sure if this is the right way to deal with this or not. It seemed that the enablement of the action maintained by this proxy was "one behind" the actual enablement of my action, and disabling it in the ActionDelegateHandlerProxy#execute method is what hoses the keybinding (there are two issues - the "one behind" issue, as well as the "forever disabled" issue). Calling refreshEnablement() as the first step whenever ActionDelegateHandlerProxy#isEnabled is called clears up both issues, but could involve a performance hit for applications other than mine. I'm not sure. I'll let the experts decide. ;o)
Any thoughts on this bug? We are still having this issue on Eclipse 3.2.1. Yes, this looks like it's part of the more general problems introduced by the CommandLegacyActionWrapper ... it's part of a bridge back to the menu item that wasn't completely connected. I've been investigating it under bug 151612 We're working to finish the new Menu story in 3.3, which will have the menu items/toolbar items going through the same framework as the keybindings (and eliminate the CommandLegacyActionWrapper and its disconnect). The plan item is bug 154130 Later, PW This item has been fixed/superceded by the menu contribution mechanism - bug 154130 ActionDelegateHandlerProxy updates the enablement of the action delegate while responding to both isEnabled and execute requests PW |