| Summary: | [KeyBindings] Cut/Copy/Paste/SelectAll don't work in Swing widgets in dialogs | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | David Green <greensopinion> |
| Component: | UI | Assignee: | Douglas Pollock <douglas.pollock> |
| Status: | RESOLVED FIXED | QA Contact: | |
| Severity: | normal | ||
| Priority: | P1 | CC: | cturner, greensopinion, hanys, Silenio_Quarti |
| Version: | 3.0 | ||
| Target Milestone: | 3.0.1 | ||
| Hardware: | All | ||
| OS: | All | ||
| Whiteboard: | |||
| Attachments: | |||
|
Description
David Green
This happens because Eclipse Workbench is intersepting these keys. A workaround is to register your own handler for cut/copy/paste, register it when the Swing component is active, and have it forward cut/copy/paste to the underlying swing widget. What eclipse interfaces would I use to "register your own handler for cut/copy/paste"? Also is there any intention of fixing this defect? This is a pretty major issue for anyone doing Swing integration into eclipse. Cut/Copy/Paste/Undo/Redo etc. are all fundamentally required for any useful UI. I'm prepared to help -- Could someone point me in the right direction to investigate this defect? What classes are involved with key bindings, and where can I find documentation or eclipse code on keystroke handling? I'd recommend looking at WidgetMethodHandler (and its descendent(s)). Make sure that they handle the case where the underlying widget is not an SWT widget. This may not be possible. An API change to WorkbenchCommandSupport might also be a solution, but this would be too much for the 3.0 time frame. What component is responsible for "stealing" keystrokes from the widgets? (Keystrokes for which there is a command are never "seen" by the underlying widget using a KeyListener) Display.addFilter WorkbenchKeyboard WorkbenchContextSupport WorkbenchCommandSupport I just sat down with Steve and went through a bit of how this works. Apparently, a composite with the embedded bit set is reported as having focus whenever an AWT/Swing control really has focus. This embedded bit is also used for things like the browser widget. Does this mean that the embedded composite should receive these commands? I believe the global key filter should not enabled when an embedded composite has focus. Or, at least, it shouldn't eat key events. The following code change to WorkbenchKeyboard.java seems to do the trick.
Notice how processing key events is avoided for any widget with the embedded
style.
I'm not sure if this change would have any other implications, however it
seems to work: the Swing widgets process the cut/copy/paste commands properly,
and other SWT widgets remain unaffected.
The only remaining work to be done would be to update the AWT/Swing key
mappings to match the workbench settings (this could be left to the
application code).
void processKeyEvent(List keyStrokes, Event event) {
// Dispatch the keyboard shortcut, if any.
boolean eatKey = false;
if (!keyStrokes.isEmpty()) {
// only attempt to process keystrokes for widgets that are
// not embedded.
if ((event.widget.getStyle() & SWT.EMBEDDED) == 0) {
try {
eatKey = press(keyStrokes, event);
} catch (CommandException e) {
logException(e);
eatKey = true;
}
} else {
if (DEBUG && DEBUG_VERBOSE) {
System.out.println("skipping embedded widget");
}
}
}
if (eatKey) {
switch (event.type) {
case SWT.KeyDown:
event.doit = false;
break;
case SWT.Traverse:
event.detail = SWT.TRAVERSE_NONE;
event.doit = true;
break;
default:
}
event.type = SWT.NONE;
}
}
Also see Bug 56223. Are you guys changing the keys for COPY/CUT/PASTE in RC2 in order to avoid this problem? If that's the case, it's not going to work out for everyone because a lot of us are already used to the original CTL-C/X/V. Please ignore my last message. Sorry. Created attachment 12342 [details]
Patch to WidgetMethodHandler
The idea of turning of key bindings in a Swing widget isn't going to work
perfectly well. The problem is that the vast majority of Eclipse key bindings
should still work (e.g., Open Type, Ctrl+Shift+T).
The problem is that the global cut, copy, paste and selectAll methods rely on
looking up the required methods on the current focus control. In the case of
Swing, the focus control is a Composite. My thought is that when this happens,
it might be better to try to look up the focusOwner from Swing's focus manager.
Then it would be possible to dispatch the cut, copy, paste and selectAll (if
appropriate).
However, this has a couple of problems right now:
+ This patch seems to cause long hangs when looking up the methods (major).
+ This technique will miss AWT components which provided no such methods (e.g.,
TextField). This I feel is a minor problem, as I believe AWT components are
rarely used (compared to Swing components).
Created attachment 12344 [details]
Project providing a test case
This plug-in project provides a view called "Swing View", which contains some
embedded Swing components. The top two text fields are a JTextField (left) and
TextField (right).
In I200406170800, using the test case I have, the cut/copy/paste key bindings work as expected. Can you confirm? While I'm not entirely sure what the problem might have been, my test case now works for me. Moving to WORKSFORME. The test case for defect 60934 (https://bugs.eclipse.org/bugs/show_bug.cgi? id=60934) demonstrates the use of some simple swing text items in a wizard dialog where the cut/copy/paste keys don't work. Please see 60934 attachment https://bugs.eclipse.org/bugs/attachment.cgi? id=10268&action=view This is happening only in dialogs opened with Swing/AWT component inside of them. The problem is as described in comment #15, but only applies to dialogs (where the WidgetMethodHandler is meant to take over). This is a showstopper issue for anyone integrating Swing into eclipse wizard dialogs... is there any way to have this issue resolved? I'm prepared to contribute. If there is a preferred fix over my previous post ( bug 63235, comment 11 ) could someone point me in the right direction? The direction taken in comment #15 might be preferrable. Otherwise, your approach might work, but it would have to be restricted to the particular case that matters (e.g., special case when the WidgetMethodHandler is about to eat a key when a Swing component has focus). The direction taken in comment 15 looks great... the only problem that I could see with it is that the method call to selectAll/copy/paste for swing components doesn't occur on the swing dispatch thread. I imagine that it should be fairly easy to eliminate the performance issue. Would it make sense to dispatch the method call to a runnable on the swing event thread? On the other hand, for the solution in comment 11, it's already restricted to the embedded style. Does it need restricting further? My concern is that disabling this key binding for all embedded widgets may not be what we want to do. There are a lot of situations other than embedded SWT widgets in dialogs that would be affected by this. While I can't think of any reason why this might be a problem, I would prefer a solution that only affected Swing widgets in dialogs. Switching it to a Swing thread may fix the problems. Created attachment 13123 [details] A version of WidgetMethodHandler based on comment 15 that appears to fix the problem Additional work put into the patch referenced in comment 15 that appears to fix the performance issue. Cut/Copy/paste works very nicely. Tested on JTextField, editable JTable, editable JComboBox. Created attachment 13124 [details] A version of WidgetMethodHandler based on comment 15 that appears to fix the problem This version fixes the infinite loop issue. I've now completed my testing the WidgetMethodHandler for both SWT and Swing items, and it looks good. This implementation is based on the patch sumbitted by Douglas Pollock (comment 15). The loop and performance issue was caused by a loop that called Class.getMethod (name,args) while traversing up the inheritance hierarcy. After reading the javadocs for Class.getMethod, this is unnecessary since Class.getMethod already provides this functionality. I'll try to review and test the patch, and (assuming everything is good) I'm suggesting this for inclusion in 3.0.1. Is there anything else that I can do to help? (has the patch been committed?) I've been away for the past three weeks. The patch has not been committed. I've modified the patch a little bit more -- just some clean up, and removing another unnecessary tree climbing piece of code. I also tried to make it so that proper error reporting happens if the Swing method fails to execute. I did this with a synchronous call to Swing. I tested with three different pieces of Eclipse. The test is simply to cut from the piece being tested, and paste to another application or to another location within Eclipse. First of all, I tried several of the built-in views. This included other embedded views, such as the Javadoc view. It worked in all of these cases. I then tried the test case that I provided as an attachment to this bug. This case does not trigger the WidgetMethodHandler. There are copy/cut/paste/selectAll RetargetActions defined in the workbench. When they have no handler, they fall back to the native behaviour. Lastly, I tested the wizard in the Bug 60934. This wizard worked without problem. I tested select all, delete, copy and paste. Billy brought up one interesting point: it might be possible for this to cause a deadlock if Eclipse owns the clipboard and Swing tries to get it. It doesn't actually deadlock; it times out after about one second, and the paste (in Swing) fails. So, I've left it as an asynchronous execution and -- in the event of failure -- it communicates back to the SWT event thread, and tells the workbench to handle the exception. This patch has been reviewed by Billy, and will be committed to the 3.0.1 branch. It should appear in the next 3.1 integration build (I200408170800) and the next 3.0.1 maintenance build (I200408180800). Please download one (or both) of these builds as they become available and confirm the fix. Works for me with build 200408180800 (3.0.1) Thanks! Verified by reporter in M200408180800. Pressing CTRL+A in an embedded JTextComponent causes an exception. The expected behaviour is for all text to be selected (Select All). After inspecting the code in a debugger it was apparent that SelectAllHandler has found the appropriate method to call (JTextComponent.selectAll) however SelectAllHandler is attempting to invoke the method on an SWT composite (the focus control). The stack trace is as follows: java.lang.IllegalArgumentException: object is not an instance of declaring class at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method) at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39) at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25) at java.lang.reflect.Method.invoke(Method.java:589) at org.eclipse.ui.internal.handlers.SelectAllHandler.execute(SelectAllHandler.java:50) at org.eclipse.ui.internal.handlers.HandlerProxy.execute(HandlerProxy.java:151) at org.eclipse.core.commands.Command.executeWithChecks(Command.java:460) at org.eclipse.core.commands.ParameterizedCommand.executeWithChecks(ParameterizedCommand.java:421) at org.eclipse.ui.internal.handlers.HandlerService.executeCommand(HandlerService.java:160) at org.eclipse.ui.internal.keys.WorkbenchKeyboard.executeCommand(WorkbenchKeyboard.java:466) at org.eclipse.ui.internal.keys.WorkbenchKeyboard.press(WorkbenchKeyboard.java:798) at org.eclipse.ui.internal.keys.WorkbenchKeyboard.processKeyEvent(WorkbenchKeyboard.java:845) at org.eclipse.ui.internal.keys.WorkbenchKeyboard.filterKeySequenceBindings(WorkbenchKeyboard.java:563) at org.eclipse.ui.internal.keys.WorkbenchKeyboard.access$3(WorkbenchKeyboard.java:506) at org.eclipse.ui.internal.keys.WorkbenchKeyboard$KeyDownFilter.handleEvent(WorkbenchKeyboard.java:122) at org.eclipse.swt.widgets.EventTable.sendEvent(EventTable.java:66) at org.eclipse.swt.widgets.Display.filterEvent(Display.java:978) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:924) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:949) at org.eclipse.swt.widgets.Widget.sendEvent(Widget.java:934) at org.eclipse.swt.widgets.Widget.sendKeyEvent(Widget.java:962) at org.eclipse.swt.widgets.Widget.sendKeyEvent(Widget.java:958) at org.eclipse.swt.widgets.Widget.wmChar(Widget.java:1272) at org.eclipse.swt.widgets.Control.WM_CHAR(Control.java:3336) at org.eclipse.swt.widgets.Control.windowProc(Control.java:3236) at org.eclipse.swt.widgets.Display.windowProc(Display.java:3965) at org.eclipse.swt.internal.win32.OS.DispatchMessageW(Native Method) at org.eclipse.swt.internal.win32.OS.DispatchMessage(OS.java:1799) at org.eclipse.swt.widgets.Display.messageProc(Display.java:2538) at org.eclipse.swt.internal.win32.OS.DispatchMessageW(Native Method) at org.eclipse.swt.internal.win32.OS.DispatchMessage(OS.java:1799) at org.eclipse.swt.widgets.Display.readAndDispatch(Display.java:2905) at org.eclipse.jface.window.Window.runEventLoop(Window.java:811) at org.eclipse.jface.window.Window.open(Window.java:789) After inspecting the code for SelectAllHandler, it was also apparent that the code will not handle embedded Swing components properly. The SelectAllHandler code should be modified to work the same as WidgetMethodHandler (its super class), which tests for the SWT.EMBEDDED style and invokes the method on the Swing event dispatch thread. BTW, this latest test occurred on 3.2M5a Please open a separate bug. I've created a seperate log Bug 132215 |