Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 63235

Summary: [KeyBindings] Cut/Copy/Paste/SelectAll don't work in Swing widgets in dialogs
Product: [Eclipse Project] Platform Reporter: David Green <greensopinion>
Component: UIAssignee: 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 Flags
Patch to WidgetMethodHandler
none
Project providing a test case
none
A version of WidgetMethodHandler based on comment 15 that appears to fix the problem
none
A version of WidgetMethodHandler based on comment 15 that appears to fix the problem none

Description David Green CLA 2004-05-20 11:56:55 EDT
When running Swing components in an SWT container, cut/copy/paste actions via 
the keyboard no longer work.  (On Windows, CTRL-C, CTRL-X, CTRLV)

This defect is present in build M8 and in build 20040519
Comment 1 Silenio Quarti CLA 2004-05-20 13:13:30 EDT
This happens because Eclipse Workbench is intersepting these keys.
Comment 2 Douglas Pollock CLA 2004-05-20 16:10:56 EDT
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. 
Comment 3 David Green CLA 2004-05-31 20:09:08 EDT
What eclipse interfaces would I use to "register your own handler for 
cut/copy/paste"?

Also is there any intention of fixing this defect?
Comment 4 David Green CLA 2004-06-01 15:50:25 EDT
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?
Comment 5 Douglas Pollock CLA 2004-06-02 09:23:38 EDT
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.
Comment 6 David Green CLA 2004-06-02 11:21:54 EDT
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)
Comment 7 Douglas Pollock CLA 2004-06-02 15:00:12 EDT
Display.addFilter
WorkbenchKeyboard
WorkbenchContextSupport
WorkbenchCommandSupport
Comment 8 Douglas Pollock CLA 2004-06-03 10:40:35 EDT
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. 
Comment 9 David Green CLA 2004-06-10 11:59:38 EDT
Does this mean that the embedded composite should receive these commands?
Comment 10 Douglas Pollock CLA 2004-06-10 12:03:09 EDT
I believe the global key filter should not enabled when an embedded composite 
has focus.  Or, at least, it shouldn't eat key events. 
 
 
Comment 11 David Green CLA 2004-06-10 12:52:53 EDT
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;
        }
    }
Comment 12 Douglas Pollock CLA 2004-06-11 12:09:05 EDT
Also see Bug 56223. 
Comment 13 Mike Han CLA 2004-06-13 09:43:17 EDT
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.
Comment 14 Mike Han CLA 2004-06-13 10:21:22 EDT
Please ignore my last message. Sorry.
Comment 15 Douglas Pollock CLA 2004-06-16 19:30:51 EDT
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).
Comment 16 Douglas Pollock CLA 2004-06-16 19:39:17 EDT
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).
Comment 17 Douglas Pollock CLA 2004-06-17 12:38:44 EDT
In I200406170800, using the test case I have, the cut/copy/paste key bindings 
work as expected. 
 
Can you confirm? 
Comment 18 Douglas Pollock CLA 2004-06-17 15:10:20 EDT
While I'm not entirely sure what the problem might have been, my test case now 
works for me.  Moving to WORKSFORME. 
Comment 19 David Green CLA 2004-06-24 20:41:44 EDT
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
Comment 20 Douglas Pollock CLA 2004-06-29 15:39:59 EDT
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). 
 
Comment 21 David Green CLA 2004-07-05 18:59:19 EDT
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?
Comment 22 Douglas Pollock CLA 2004-07-06 08:45:30 EDT
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). 
Comment 23 David Green CLA 2004-07-07 11:23:33 EDT
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?

Comment 24 Douglas Pollock CLA 2004-07-07 12:16:15 EDT
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. 
Comment 25 David Green CLA 2004-07-09 19:47:21 EDT
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.
Comment 26 David Green CLA 2004-07-09 21:12:05 EDT
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.
Comment 27 David Green CLA 2004-07-09 21:15:09 EDT
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.
Comment 28 Douglas Pollock CLA 2004-07-10 14:20:15 EDT
I'll try to review and test the patch, and (assuming everything is good) I'm 
suggesting this for inclusion in 3.0.1. 
Comment 29 David Green CLA 2004-07-30 18:44:51 EDT
Is there anything else that I can do to help?  (has the patch been committed?)
Comment 30 Douglas Pollock CLA 2004-08-09 13:51:38 EDT
I've been away for the past three weeks.  The patch has not been committed. 
Comment 31 Douglas Pollock CLA 2004-08-13 11:14:56 EDT
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. 
 
Comment 32 David Green CLA 2004-08-23 15:21:12 EDT
Works for me with build 200408180800 (3.0.1)
Thanks!
Comment 33 Douglas Pollock CLA 2004-08-24 13:46:30 EDT
Verified by reporter in M200408180800. 
Comment 34 David Green CLA 2006-03-16 11:28:53 EST
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.

Comment 35 David Green CLA 2006-03-16 11:29:32 EST
BTW, this latest test occurred on 3.2M5a
Comment 36 Douglas Pollock CLA 2006-03-16 13:12:26 EST
Please open a separate bug.
Comment 37 David Green CLA 2006-03-16 13:23:21 EST
I've created a seperate log Bug 132215