Bug 52957 - [KeyBindings] Keybinding Service consumes keystrokes for unregistered commands
Summary: [KeyBindings] Keybinding Service consumes keystrokes for unregistered commands
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.0   Edit
Hardware: PC Windows XP
: P1 major (vote)
Target Milestone: 3.0 M9   Edit
Assignee: Douglas Pollock CLA
QA Contact:
URL:
Whiteboard:
Keywords: accessibility
Depends on:
Blocks:
 
Reported: 2004-02-24 11:50 EST by Randy Hudson CLA
Modified: 2004-05-19 16:55 EDT (History)
4 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Randy Hudson CLA 2004-02-24 11:50:01 EST
My editor has no "Content Assist" action handler.
My editor's canvas does not receive CTRL+SPACE KeyPressed event.

From mailing list:

> I'm pretty sure Nick said that if the action bound to a keybinding is
> disabled (or bound to NULL), then the keybinding service would stop
> consuming that key.  But this is not the behavior we are seeing.  For

We check whether the command has a handler (i.e., action) associated with it. 
If it doesn't, then we don't eat the key.  We eat all keys that: (1) have a 
key binding currently in context; and (2) have a handler currently active.


> example, we also need to receive CTRL+SPACE, which has universal meaning
> of "toggle selection" of item with focus. But the keybinding for Content
> Assist is getting in the way. Why do native Trees and Tables receive
> CTRL+SPACE, but our Canvas does not?

Likely because a handler is registered for your part.  I'm just guessing, as
I'm not sure of the details of your set-up.

Nope, we are not registering a handler
-Randy
Comment 1 Douglas Pollock CLA 2004-02-24 12:01:54 EST
I need the following information from you: 
+ The build you are running 
+ The console output when running with the key binding debugging information.  
This is available in the options file for Platform UI.  Please set both the 
debugging and verbose debugging flags to true. 
 
You are not experiencing "crashes, loss of data, severe memory leak", so 
please do not mark the bug as critical.  Downgrading to "major".... 
Comment 2 Randy Hudson CLA 2004-02-24 16:50:05 EST
The PDE launcher is still busted M7 so I cannot enable tracing.  Anyway, you 
can reproduce quite easily.

Here are the steps reproduce. Use the PDE wizard to create a new plug-in 
project.  Choose the template "plug-in with View", and then choose to ues a 
TreeViewer (which will be deleted later), and deselect all of the unrelated 
stuff like toolbars, context menus, filters, sorting, whatever.

Now, open the generated class "SampleView", and change it to read:

public class SampleView extends ViewPart {
public void createPartControl(Composite parent) {
  new Text(parent, SWT.MULTI | SWT.V_SCROLL)
      .addKeyListener(new KeyAdapter() {
    public void keyPressed(KeyEvent e) {
      if (e.character == ' ' && e.stateMask == SWT.CTRL)
        ((Text)e.widget).append("Ctrl+SPACE");
    }
});
}
public void setFocus() {
}
}

Run the runtime workbench and note that CTRL+SPACE is not being received by 
the view's Text control.  Also, not that COPY/CUT/PASTE do not work either, 
yet the view has not provided and action handlers for *any* of these actions.
Comment 3 Chris McLaren CLA 2004-02-24 17:13:31 EST
a comment in passing (i.e. i didn't think too much about it, so take it with a grain of salt.. ):

cut, copy, and paste are 'global' actions, so they are registered on startup (opening a workbench 
window). [search backwards from WorkbenchWindow.registerGlobalAction to 
org.eclipse.internal.ide.WorkbenchActionBuilder.makeActions to see this unfold..]

that means, for global or 'retargetable' actions, the command manager always thinks there is a handler. 
(i don't think the workbench window never unregisters global actions.)

if a RetargetAction doesn't have a handler in the active part, it simply says its disabled. that doesn't 
help us down at the command manager. the workbench is not making the distinction between 
'unhandled' and 'handled but disabled'.

not sure about content assist.. maybe the same problem. i don't think ui registers content assist as a 
global action, though.

anyway, food for thought..
Comment 4 Randy Hudson CLA 2004-02-24 18:01:14 EST
Yup, I was thinking the same thing, which is why I decided not to start off by 
saying "come on guys, this is so obvious ... cut/copy/paste has been busted for 
ages" :-)
Comment 5 Randy Hudson CLA 2004-02-27 15:20:22 EST
So, the second form of this problem is that keystrokes are consumed for 
commands which ARE registered with a Global Action, BUT for which there is no 
Action Handler.  This has been around forever, and it is why 
Undo/Redo/Cut/Copy/Paste do not work inside simple Text controls.
Comment 6 Douglas Pollock CLA 2004-02-27 15:45:05 EST
Confirmed on Randy's reproduce case from comment #2. 
 
Key binding service recognizes the command identifier 
"org.eclipse.ui.edit.copy", but replies that the command is disabled. 
 
Chris' guess was an alarmingly good one. 
 
I would suggested making getActionHandler public on RetargetAction.  The class 
is flagged as not intended to be subclassed, and there are only ten 
descendents in Eclipse.  :)  None of them attempt to override the behaviour of 
this method. 
 
Now, in ActionHandler, we can make one of those icky downcasts to 
RetargetAction when appropriate.  This will allow us to distinguish between 
the disabled and unhandled cases of RetargetAction. 
 
 
Alternatively, we can make it so that disabled actions don't eat key bindings.  
This is more direct, but I think the semantic is backward somehow.  I would 
expect that if some command were disabled that it wouldn't be allowed to 
continue to the widget hierarchy -- where it might be interpreted as something 
else altogether. 
 
 
How do people feel about these options? 
Comment 7 Douglas Pollock CLA 2004-03-24 14:10:57 EST
This is being deferred from M8, and will be fixed for the first integration 
build in M9.  The reasons for deferral at this point is that: 
+ It's too close to M8. 
+ While it does not seem risky, it could have unintentional side effects. 
+ This bug has not been noticed very often. 
 
 
It will be ready for I200403300800. 
Comment 8 Douglas Pollock CLA 2004-03-29 15:59:58 EST
I believe this bug is now fixed in CVS.  There were two separate problems.  
 
First of all, I've expanded RetargetAction to expose its internal handler.  
ActionHandler and Command now use this extra information (if present) to decide 
whether a command is *really* handled. 
 
Secondly, the perfect match branch of the press method seemed to contain a 
logic error.  It said to swallow the key if there *weren't* previous key 
strokes and a perfect match occurred.  I believe it was intended to be if there 
*were* previous key strokes.  In general, though, it might be better to remove 
that check all together.  Really, it might be better if the defined/handled 
state of a command was all that controlled the eating of keys. 
 
Let me know if you see this problem again/still post-I200403300800. 
 
Comment 9 Douglas Pollock CLA 2004-05-19 16:55:28 EDT
Verified in I200405190010 by using the test case in Comment #2.  Solaris 8,
Motif, CDE.