Community
Participate
Working Groups
The class org.eclipse.ui.internal.handlers.WidgetMethodHandler uses some focus API from before JRE 1.4, namely javax.swing.FocusManager. The implementation of this class and the DefaultFocusManager have these javadoc comments: * This class has been obsoleted by the 1.4 focus APIs. While client code may * still use this class, developers are strongly encouraged to use * <code>java.awt.KeyboardFocusManager</code> and * <code>java.awt.DefaultKeyboardFocusManager</code> instead. Please see the * Focus Specification for more information. Additionally, the DefaultFocusManager installs an instance of LegacyGlueFocusTraversalPolicy, which causes endless recursions when other plugins use the JRE 1.4 focus APIs. We have seen such endless recursions in the context of the Eclipse Albireo project (http://wiki.eclipse.org/Albireo_Project): ... java.awt.ContainerOrderFocusTraversalPolicy.getLastComponent(ContainerOrderFocusTraversalPolicy.java:366) javax.swing.DefaultFocusManager.getLastComponent(DefaultFocusManager.java:119) javax.swing.LegacyGlueFocusTraversalPolicy.getLastComponent(LegacyGlueFocusTraversalPolicy.java:124) java.awt.ContainerOrderFocusTraversalPolicy.getLastComponent(ContainerOrderFocusTraversalPolicy.java:366) ... It would be better if Eclipse did not cause this LegacyGlueFocusTraversalPolicy to be installed. Patch attached.
Created attachment 97624 [details] tentative fix
The stack trace of the activation of this troublesome LegacyGlueFocusTraversalPolicy is like this: LegacyGlueFocusTraversalPolicy.<init>(DefaultFocusManager) line: 41 DelegatingDefaultFocusManager(DefaultFocusManager).<init>() line: 34 DelegatingDefaultFocusManager.<init>(KeyboardFocusManager) line: 26 FocusManager.getCurrentManager() line: 62 NativeMethodAccessorImpl.invoke0(Method, Object, Object[]) line: not available [native method] NativeMethodAccessorImpl.invoke(Object, Object[]) line: 39 DelegatingMethodAccessorImpl.invoke(Object, Object[]) line: 25 Method.invoke(Object, Object...) line: 585 WidgetMethodHandler.getFocusComponent() line: 175 WidgetMethodHandler.getMethodToExecute() line: 217 WidgetMethodHandler.isEnabled() line: 184
Created attachment 98914 [details] Widget fix v02 Minor updates including a copyright update PW
Kim, could you please review for a +1 Thanx, PW
Just to be clear, all of the interesting changes are in getFocusComponent()? There are a lot of formatting changes in that patch... as for getFocusComponent(), is it possible for any of the calls to KeyboardFocusManager to return null? Should there be a try/catch there as well with a graceful degradation to the old behaviour?
(In reply to comment #5) > Just to be clear, all of the interesting changes are in getFocusComponent()? > There are a lot of formatting changes in that patch... Nuts, I must have ignore whitespace turned on by default. > as for getFocusComponent(), is it possible for any of the calls to > KeyboardFocusManager to return null? Should there be a try/catch there as well > with a graceful degradation to the old behaviour? > It is possible for getFocusOwner() to return null ... but that indicates there is no component in focus and it is valid for us to return null from the method. I'm pretty sure we don't want to fall back to the old way (which should also return null in this case). PW
(In reply to comment #6) > (In reply to comment #5) > > Just to be clear, all of the interesting changes are in getFocusComponent()? > > There are a lot of formatting changes in that patch... > > Nuts, I must have ignore whitespace turned on by default. > > > as for getFocusComponent(), is it possible for any of the calls to > > KeyboardFocusManager to return null? Should there be a try/catch there as well > > with a graceful degradation to the old behaviour? > > > > It is possible for getFocusOwner() to return null ... but that indicates there > is no component in focus and it is valid for us to return null from the method. > I'm pretty sure we don't want to fall back to the old way (which should also > return null in this case). > > > PW > I was more concerned with getCurrentKeyboardFocusManager...
(In reply to comment #7) > > I was more concerned with getCurrentKeyboardFocusManager... > No, that instantiates and/or returns the singleton PW
+1
Bruno, could you verify this on Thursday using the I20080507-2000 I build? Released to HEAD PW
