Community
Participate
Working Groups
The purpose is to bind some keys to some action bar actions used by a view (org.eclipse.ui.ViewPart). The view part is used both within a workbench view and a modal dialog. For the modal dialog I've defined a custom context (see below) and, before the dialog executes I try to activate my custom context and link the commands associated with the key bindings to the view actions. It works very well up until 3.8 (I never tested for 4.0 and 4.1). In 4.2 I get key binding conflict warning when I activate my context and even if I remove the conflicting bindings (for F2, F3 and F4), although the commands are correctly associated with the view actions (the accelerator appears within the tool-tip), pressing the shortcut keys still has no effect. The code for dialog configuration: class MyDialog extends TitleAreaDialog { protected Control createDialogArea(Composite parent) { Composite area = (Composite) super.createDialogArea(parent); ... //The toolbar used for view actions: ToolBar toolBar = new ToolBar(area, SWT.FLAT | SWT.RIGHT); mnuMgr = new ToolBarManager(toolBar); ... Composite container = new Composite(area, SWT.NONE); ... cv = new CustomView(mnuMgr); //the ViewPart cv.createPartControl(container); area.setTabList(new Control[]{toolBar, container}); //Activate my custom context (see code below) cv.hookKeyboard(); area.addDisposeListener( new DisposeListener() { @Override public void widgetDisposed(DisposeEvent e) { ViewManager.INSTANCE.unRegisterViewPart(cv); //deactivate the custom context: cv.releaseKeyboard(); } }); return area; } ... } public class CustomView extends ViewPart { public static final String CONTEXT_ID = "com.example.my.custom.context" ... public void hookKeyboard() { if (isDialogMode && activation == null) { IContextService contextService = (IContextService) PlatformUI.getWorkbench().getService(IContextService.class); ICommandService commandService = (ICommandService) PlatformUI.getWorkbench().getService(ICommandService.class); contextService.registerShell(shell, IContextService.TYPE_DIALOG); //THIS IS THE POINT WHERE THE KEY BINDING CONFLICT IS ISSUED: activation = contextService.activateContext(CONTEXT_ID); //Associating the defined commands with the dialog defined //handler (see below) Command c = commandService.getCommand(CMD_ACTION_DEL); c.setHandler(handler); c = commandService.getCommand(CMD_ACTION_MOD); c.setHandler(handler); c = commandService.getCommand(CMD_ACTION_ADD); c.setHandler(handler); } } public void releaseKeyboard() { if (isDialogMode && activation != null) { IContextService contextService = (IContextService) PlatformUI.getWorkbench().getService(IContextService.class); contextService.deactivateContext(activation); activation = null; } } ... public class Handler extends AbstractHandler { // THIS IS NEVER EXECUTED IN 4.2 ! @Override public Object execute(ExecutionEvent event) throws ExecutionException { if (inCommand) return null; inCommand = true; try { if (event.getCommand().getId().equals(CMD_ACTION_DEL)) actionDelete.run(); //view action else if (event.getCommand().getId().equals(CMD_ACTION_MOD)) actionEdit.run(); //view action else if (event.getCommand().getId().equals(CMD_ACTION_ADD)) actionAdd.run(); //view action } finally { inCommand = false; } return null; } } final AbstractHandler handler = new Handler(); ... } Context definition in plugin.xml: <extension point="org.eclipse.ui.contexts"> <context id="com.example.my.custom.context" name="My custom context" parentId="org.eclipse.ui.contexts.dialog"> </context> </extension> Binding definitions: <extension point="org.eclipse.ui.bindings"> <key commandId="com.example.my.view.edit" contextId="com.example.my.custom.context" schemeId="org.eclipse.ui.defaultAcceleratorConfiguration" sequence="F3"> </key> <key commandId="com.example.my.view..add" contextId="com.example.my.custom.context" schemeId="org.eclipse.ui.defaultAcceleratorConfiguration" sequence="F2"> </key> <key commandId="com.example.my.view.delete" contextId="com.example.my.custom.context" schemeId="org.eclipse.ui.defaultAcceleratorConfiguration" sequence="F4"> </key> </extension> Command definitions: <extension point="org.eclipse.ui.commands"> <category description="My Dialog Actions" id="com.example.category.dialog.actions" name="Dialog actions"> </category> <command categoryId="com.example.category.dialog.actions" id="com.example.dialog.add" name="New..."> </command> <command categoryId="com.example.category.dialog.actions" id="com.example.dialog.edit" name="Edit..."> </command> <command categoryId="com.example.category.dialog.actions" id="com.example.dialog.delete" name="Delete"> </command> </extension> The conflicting bindings (notice the different contexts "org.eclipse.ui.contexts.window" and "com.example.my.custom.context" : ENTRY org.eclipse.jface 2 0 2012-08-24 10:54:50.641 !MESSAGE Keybinding conflicts occurred. They may interfere with normal accelerator operation. !SUBENTRY 1 org.eclipse.jface 2 0 2012-08-24 10:54:50.641 !MESSAGE A conflict occurred for F4: Binding(F4, ParameterizedCommand(Command(org.eclipse.jdt.ui.edit.text.java.open.type.hierarchy,Open Type Hierarchy, Open a type hierarchy on the selected element, Category(org.eclipse.ui.category.navigate,Navigate,null,true), org.eclipse.ui.internal.MakeHandlersGo@282c55f1, ,,true),null), org.eclipse.ui.defaultAcceleratorConfiguration, org.eclipse.ui.contexts.window,,,system) Binding(F4, ParameterizedCommand(Command(com.example.dialog.delete,Delete, , Category(com.example.category.dialog.actions,Dialog actions,null,true), com.example.views.CustomView$Handler@76be55d1, ,,true),null), org.eclipse.ui.defaultAcceleratorConfiguration, com.example.my.custom.context,,,system) !SUBENTRY 1 org.eclipse.jface 2 0 2012-08-24 10:54:50.641 !MESSAGE A conflict occurred for F2: Binding(F2, ParameterizedCommand(Command(org.eclipse.ui.edit.rename,Rename, Rename the selected item, Category(org.eclipse.ui.category.file,File,null,true), org.eclipse.ui.internal.MakeHandlersGo@58fe210a, ,,true),null), org.eclipse.ui.defaultAcceleratorConfiguration, org.eclipse.ui.contexts.window,,,system) Binding(F2, ParameterizedCommand(Command(com.example.dialog.add,New..., , Category(com.example.category.dialog.actions,Dialog actions,null,true), com.example.views.CustomView$Handler@76be55d1, ,,true),null), org.eclipse.ui.defaultAcceleratorConfiguration, com.example.my.custom.context,,,system) !SUBENTRY 1 org.eclipse.jface 2 0 2012-08-24 10:54:50.641 !MESSAGE A conflict occurred for F3: Binding(F3, ParameterizedCommand(Command(org.eclipse.jdt.ui.edit.text.java.open.editor,Open Declaration, Open an editor on the selected element, Category(org.eclipse.ui.category.navigate,Navigate,null,true), org.eclipse.ui.internal.MakeHandlersGo@49f4493e, ,,true),null), org.eclipse.ui.defaultAcceleratorConfiguration, org.eclipse.ui.contexts.window,,,system) Binding(F3, ParameterizedCommand(Command(com.example.dialog.edit,Edit..., , Category(com.example.category.dialog.actions,Dialog actions,null,true), com.example.views.CustomView$Handler@76be55d1, ,,true),null), org.eclipse.ui.defaultAcceleratorConfiguration, com.example.my.custom.context,,,system)
There are 2 problems here. You should not be getting the conflict warning for your context off of the dialog context, because they have different parents and you correctly register the shell as a dialog. But setting handlers directly on the Command objects won't work, as they're not used as part of the command framework processing in 4.2. By setting a handler directly on the command, you code is saying it wants to circumvent the workbench command framework and "do it myself" as it were. I would recommend against it, but I think some changes I plan in 4.3 will allow that to continue working as well. PW
(In reply to comment #1) > But setting handlers directly on the Command objects won't work, as they're > not used as part of the command framework processing in 4.2. By setting a > handler directly on the command, you code is saying it wants to circumvent > the workbench command framework and "do it myself" as it were. > > I would recommend against it, but I think some changes I plan in 4.3 will > allow that to continue working as well. It's indeed a "do it myself" job here: it's because my plugin uses the now deprecated "actionSets" extension point (as I'm sure many others still do) and I need to link to the key binding infrastructure manually. I plan to switch to the "menus" and "commands" approach as "actionSets" and "popupMenus" do have some unpleasant drawbacks, but I think it will be a good idea to still support the manual Command.setHandler() call as the upgrade takes some time and there are plugins in use that no longer work as expected with Juno... Cosmin
(In reply to comment #2) > but I think it will be a > good idea to still support the manual Command.setHandler() call as the > upgrade takes some time and there are plugins in use that no longer work as > expected with Juno... It's not really supported now. Command#setHandler(*) is API of a low level bundle used to implement the workbench command framework, but the workbench command framework (including keybindings) has no API that specs it will honour that. In 3.x, it can work more or less (as any change in a set of source providers can override the setHandler(*) and reset it even in 3.x). In 4.x, it was circumvented, but there's enough good in it that I think it (calls to setHandler from the framework) should creep back into use ... but calling setHandler(*) from one's own plugin still works (or not) with no guarantees. PW
I see the same on our plugin where the non-texteditor conflicts with the texteditor keybindings. This is strange our keybinding-context doesn't inherite from the the texteditor-context. Is it the same using Command.setHandler directly or going using the HandlerService#activateHandler(commandId, handler)? If this is not the way to go, what's supposed to be the right way in eclipse 4.2 doing this? Does this mean that using the 4.2 way our plugins will no longer work with 3.7/3.8?
(In reply to comment #4) > I see the same on our plugin where the non-texteditor conflicts with the > texteditor keybindings. This is strange our keybinding-context doesn't > inherite from the the texteditor-context. > > Is it the same using Command.setHandler directly or going using the > HandlerService#activateHandler(commandId, handler)? The keybindings are managed differently than handlers, and conflicts in keybindings aren't related to how handlers do things. I hope to address the keybinding conflict in this bug, as the pattern used shouldn't generate errors. > If this is not the way to go, what's supposed to be the right way in eclipse > 4.2 doing this? Does this mean that using the 4.2 way our plugins will no > longer work with 3.7/3.8? Using IHandlerService#activateHandler(*) is the preferred way that works in 3.x and 4.x. PW
Thanks Paul(In reply to comment #5) > (In reply to comment #4) > > I see the same on our plugin where the non-texteditor conflicts with the > > texteditor keybindings. This is strange our keybinding-context doesn't > > inherite from the the texteditor-context. > > > > Is it the same using Command.setHandler directly or going using the > > HandlerService#activateHandler(commandId, handler)? > > The keybindings are managed differently than handlers, and conflicts in > keybindings aren't related to how handlers do things. I hope to address the > keybinding conflict in this bug, as the pattern used shouldn't generate > errors. > > > If this is not the way to go, what's supposed to be the right way in eclipse > > 4.2 doing this? Does this mean that using the 4.2 way our plugins will no > > longer work with 3.7/3.8? > > Using IHandlerService#activateHandler(*) is the preferred way that works in > 3.x and 4.x. > > PW Many thanks for the fast response and clarifying this.
Created attachment 231268 [details] dialog with key conflict project This project exhibits the behaviour. It logs the warning, but it doesn't actually interfere with the keybinding itself (my handler for com.example.dialog.edit was executed while the dialog was up). !MESSAGE A conflict occurred for F3: Binding(F3, ParameterizedCommand(Command(org.eclipse.jdt.ui.edit.text.java.open.editor,Open Declaration, Open an editor on the selected element, Category(org.eclipse.ui.category.navigate,Navigate,null,true), org.eclipse.ui.internal.WorkbenchHandlerServiceHandler@d52ae666, ,,true),null), org.eclipse.ui.defaultAcceleratorConfiguration, org.eclipse.ui.contexts.window,,,system) Binding(F3, ParameterizedCommand(Command(com.example.dialog.edit,Edit..., , Category(com.example.category.dialog.actions,Dialog actions,My Dialog Actions,true), org.eclipse.ui.internal.WorkbenchHandlerServiceHandler@a5c82948, ,,true),null), org.eclipse.ui.defaultAcceleratorConfiguration, com.example.my.custom.context,,,system)
We can't pick the parent context, I commented on https://git.eclipse.org/r/#/c/14068 PW
(In reply to comment #8) > We can't pick the parent context, I commented on > https://git.eclipse.org/r/#/c/14068 That should be https://git.eclipse.org/r/#/c/14134/ PW
Daniel, could you please provide a brief description of the problem you found that causes this symptoms? PW
(In reply to comment #10) > Daniel, could you please provide a brief description of the problem you > found that causes this symptoms? > > PW It seems that the issue is caused by not supported case in validation the context id during resolving binding conflicts. Currently we assume that active contexts tree is continuous so we are able to find parent context of one conflicting binding equals to the context of the next binding. So when we have the following active contexts tree (tree consists of the child - parent relations): org.eclipse.ui.contexts.mycontext -> org.eclipse.ui.contexts.window, org.eclipse.ui.contexts.window -> org.eclipse.ui.contexts.dialogAndWindow, org.eclipse.ui.contexts.dialogAndWindow -> null, and following conflicts: bindingA: (M1 + 1, org.eclipse.ui.contexts.window), bindingB: (M1 + 1, org.eclipse.ui.contexts.mycontext) we select 'bindingB' as the more specific - parent context of the 'bindingB' equals to the 'bindingA' context. However in the case of the considered issue we have discontinuity in the active contexts tree, for instance: org.eclipse.ui.contexts.mycontext -> org.eclipse.ui.contexts.dialogAndWindow, org.eclipse.ui.contexts.window -> org.eclipse.ui.contexts.dialogAndWindow, org.eclipse.ui.contexts.dialogAndWindow -> null, and having following conflicts: bindingC: (M1 + 1, org.eclipse.ui.contexts.window), bindingD: (M1 + 1, org.eclipse.ui.contexts.mycontext) we are not able to resolve this conflict since we can't find any parent context of one binding equals to the context of the next binding. We report the unresolved conflict in such case and binding doesn't work. The solution proposed in the new patch relies on comparing the path length to the common context and selecting the winning binding with longest path as the more specific hopefully my explanation is edible, Daniel
(In reply to comment #11) > > However in the case of the considered issue we have discontinuity in the > active contexts tree, for instance: > org.eclipse.ui.contexts.mycontext -> > org.eclipse.ui.contexts.dialogAndWindow, > org.eclipse.ui.contexts.window -> org.eclipse.ui.contexts.dialogAndWindow, > org.eclipse.ui.contexts.dialogAndWindow -> null, > > and having following conflicts: > bindingC: (M1 + 1, org.eclipse.ui.contexts.window), > bindingD: (M1 + 1, org.eclipse.ui.contexts.mycontext) > > we are not able to resolve this conflict since we can't find any parent > context of one binding equals to the context of the next binding. > We report the unresolved conflict in such case and binding doesn't work. This is the desired behaviour. If 2 sibling contexts are active and have the same keybinding, then that key combination is in conflict. The exception to that is keys in the dialog tree and keys in the window tree are not considered in conflict. From comment #2 "You should not be getting the conflict warning for your context off of the dialog context, because they have different parents and you correctly register the shell as a dialog." Only one branch of either the dialog based contexts or the window based contexts should be active at one time (in theory). In comment #7 I have the example. We need to understand why it logs the warning for a dialog vs window context since it did not do that in 3.x (did we prune the list of active contexts from dialog and from window depending on what shell was active)? Then we need to make sure it would pick the correct keybinding if a dialog is active vs if the workbench window is active. PW
(In reply to comment #12) > (In reply to comment #11) > > > > However in the case of the considered issue we have discontinuity in the > > active contexts tree, for instance: > > org.eclipse.ui.contexts.mycontext -> > > org.eclipse.ui.contexts.dialogAndWindow, > > org.eclipse.ui.contexts.window -> org.eclipse.ui.contexts.dialogAndWindow, > > org.eclipse.ui.contexts.dialogAndWindow -> null, > > > > and having following conflicts: > > bindingC: (M1 + 1, org.eclipse.ui.contexts.window), > > bindingD: (M1 + 1, org.eclipse.ui.contexts.mycontext) > > > > we are not able to resolve this conflict since we can't find any parent > > context of one binding equals to the context of the next binding. > > We report the unresolved conflict in such case and binding doesn't work. > > This is the desired behaviour. If 2 sibling contexts are active and have > the same keybinding, then that key combination is in conflict. > > The exception to that is keys in the dialog tree and keys in the window tree > are not considered in conflict. From comment #2 "You should not be getting > the conflict warning for your context off of the dialog context, because > they have different parents and you correctly register the shell as a > dialog." Only one branch of either the dialog based contexts or the window > based contexts should be active at one time (in theory). > > In comment #7 I have the example. We need to understand why it logs the > warning for a dialog vs window context since it did not do that in 3.x (did > we prune the list of active contexts from dialog and from window depending > on what shell was active)? Then we need to make sure it would pick the > correct keybinding if a dialog is active vs if the workbench window is > active. > > PW OK, so let me jump again to this haystack to find the needle. However maybe would be worth to consider my last patch for it that fixes the final issue. Following this idea we will have to amend the logging during resolving conflicting bindings to expose this case in the log file Daniel
(In reply to comment #13) > > OK, so let me jump again to this haystack to find the needle. However maybe > would be worth to consider my last patch for it that fixes the final issue. > Following this idea we will have to amend the logging during resolving > conflicting bindings to expose this case in the log file The log is only invalid when the conflicts are in the dialog subtree and the window subtree ... if there's a conflict in the javaScope and jsScope (java editor and javascript editor but the contexts are somehow active at the same time) then that's a valid conflict that should be reported. PW
(In reply to comment #14) > (In reply to comment #13) > > > > OK, so let me jump again to this haystack to find the needle. However maybe > > would be worth to consider my last patch for it that fixes the final issue. > > Following this idea we will have to amend the logging during resolving > > conflicting bindings to expose this case in the log file > > > The log is only invalid when the conflicts are in the dialog subtree and the > window subtree ... if there's a conflict in the javaScope and jsScope (java > editor and javascript editor but the contexts are somehow active at the same > time) then that's a valid conflict that should be reported. > > PW OK, I've found the real issue that is caused by removing during E4 related refactoring the registration of the main shell with the IContextService.registerShell method. After skipping this step the 'org.eclipse.ui.contexts.window' context was never deactivated (removed from the active context list in the ContextManager) and always added to the active context tree (the BindingManager.create*ContextTreeFor methods) It produced the discontinuity on the tree mentioned in the comment 11, multiple bindings with the same key sequence were valid and the conflict was reported. The new patch the fixes the issue was sent to Gerrit - http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=3ebb45da67b9534cfbd90ffa384f6d77c931aa58 Daniel
Released as http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=0c314f51dca6c35de4cea8eb977fa5d3bf2132cb Thanks Daniel PW
Verified in the build: I20130916-2330
