Skip to main content

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [List Home]
Re: [albireo-dev] Participating more naturally in SWT tab order

Gordon Hirsch wrote:
Bruno Haible wrote:
I see that one needs to click on the tab header twice until a caret becomes
visible in one of the text fields. While on Gtk clicking on it once is enough.
Is this normal behaviour on Windows, or is it a bug similar to the first test
case in   http://wiki.eclipse.org/Albireo_Focus_Management_Test_Cases ?

The problem here is that the RCP workbench is setting focus on the
view's composite before the underlying Swing components are created, so
there is nothing to take the focus.

I've committed code to defer the focus change on calls to both setFocus
and forceFocus. This is done on both the SwingControl and the child
composite that is created in the SWT.BORDER case.

This code was more complex than necessary. I've reworked it so that setFocus and forceFocus simply operate on the SWT control once again, even if the swing component has not yet been created. Later after it has been created, focus is manually propagated to it (if necessary) as part of the SwingControl's component creation process.

I also discovered a bug with focus stealing on Windows due to unreliable results on Display.getActiveShell().

I'd like to create a second alpha release soon, but I've noticed a few other irritating problems wrt focus that need to be fixed first.


### Eclipse Workspace Patch 1.0
#P org.eclipse.albireo.core
Index: src/org/eclipse/albireo/core/SwingControl.java
===================================================================
RCS file: /cvsroot/technology/org.eclipse.albireo/org.eclipse.albireo.core/src/org/eclipse/albireo/core/SwingControl.java,v
retrieving revision 1.79
diff -u -r1.79 SwingControl.java
--- src/org/eclipse/albireo/core/SwingControl.java	30 Apr 2008 22:13:40 -0000	1.79
+++ src/org/eclipse/albireo/core/SwingControl.java	6 May 2008 19:31:50 -0000
@@ -343,6 +343,10 @@
                         display,
                         new Runnable() {
                             public void run() {
+
+                                // Propagate focus to Swing, if necesssary
+                                focusHandler.activateEmbeddedFrame();
+
                                 // Now that the preferred size is known, enable
                                 // the layout on the layoutable ancestor.
                                 if (layoutDeferredAncestor != null) {
@@ -1276,10 +1280,9 @@
      * Overridden to return false and prevent any focus change if the embedded Swing component is 
      * not focusable. 
      * <p>
-     * Note: If the Swing component has not yet been created, then this method will return <code>true</code>, 
-     * and it will defer setting the focus until after it has been created. If you need to know whether 
-     * the focus was accepted in this case, override {@link #afterComponentCreatedSWTThread()} and
-     * wait until it is called before calling this method. 
+     * Note: If the Swing component has not yet been created, then this method will temporarily set focus
+     * on its parent SWT {@link Composite}. After the Swing component is created, and if it is focusable, focus
+     * will be transferred to it.   
      */
     public boolean setFocus() {
         checkWidget();
@@ -1302,10 +1305,9 @@
      * Overridden to return false and prevent any focus change if the embedded Swing component is 
      * not focusable. 
      * <p>
-     * Note: If the Swing component has not yet been created, then this method will return <code>true</code>, 
-     * and it will defer forcing the focus until after it has been created. If you need to know whether 
-     * the focus was accepted in this case, override {@link #afterComponentCreatedSWTThread()} and
-     * wait until it is called before calling this method. 
+     * Note: If the Swing component has not yet been created, then this method will temporarily set focus
+     * on its parent SWT {@link Composite}. After the Swing component is created, and if it is focusable, focus
+     * will be transferred to it.   
      */
     public boolean forceFocus() {
         checkWidget();
@@ -1349,30 +1351,13 @@
      * @return the result of running the focus setter, or true if it was deferred
      */
     protected boolean handleFocusOperation(final RunnableWithResult focusSetter) {
+        // isFocusable() should be safe to call from the SWT thread. 
         if ((swingComponent != null) && !swingComponent.isFocusable()) {
             // Fail if the underlying swing component is not focusable
             return false;
         } else {
-            if (swingComponent == null) {
-                // Swing component may not have been created. Defer the setFocus
-                EventQueue.invokeLater(new Runnable() {
-                    public void run() {
-                        display.asyncExec(new Runnable() {
-                            public void run() {
-                                // Repeat focusable check now that the component should have been created
-                                if ((swingComponent == null) || swingComponent.isFocusable()) {
-                                    focusSetter.run();
-                                }
-                            }
-                        });
-                    }
-                });
-                return true;
-            } else {
-                // Normal case. Set the focus directly. 
-                focusSetter.run();
-                return ((Boolean)focusSetter.getResult()).booleanValue();
-            }
+          focusSetter.run();
+          return ((Boolean)focusSetter.getResult()).booleanValue();
         }
     }
     
Index: src/org/eclipse/albireo/internal/FocusHandler.java
===================================================================
RCS file: /cvsroot/technology/org.eclipse.albireo/org.eclipse.albireo.core/src/org/eclipse/albireo/internal/FocusHandler.java,v
retrieving revision 1.19
diff -u -r1.19 FocusHandler.java
--- src/org/eclipse/albireo/internal/FocusHandler.java	28 Apr 2008 17:14:36 -0000	1.19
+++ src/org/eclipse/albireo/internal/FocusHandler.java	6 May 2008 19:31:50 -0000
@@ -37,6 +37,7 @@
 import org.eclipse.swt.widgets.Display;
 import org.eclipse.swt.widgets.Event;
 import org.eclipse.swt.widgets.Listener;
+import org.eclipse.swt.widgets.Shell;
 
 public class FocusHandler {
 
@@ -55,6 +56,8 @@
     private static Method synthesizeMethod = null;
 
     private static Composite activeBorderless = null;
+    private static Composite activeShell = null;
+    private static boolean firstInstance = true;
 
     // ========================================================================
 
@@ -92,6 +95,10 @@
         display.addFilter(SWT.Activate, swtEventFilter);
         display.addFilter(SWT.Deactivate, swtEventFilter);
         display.addFilter(SWT.Traverse, swtEventFilter);
+        if (firstInstance) {
+            activeShell = display.getActiveShell();
+            firstInstance = false;
+        }
         
         // We won't get an Activate event for the newly created control, so update
         // the static tracking field here. 
@@ -311,25 +318,7 @@
         if (!display.isDisposed()) {
             ThreadingHandler.getInstance().asyncExec(display, new Runnable() {
                 public void run() {
-                    if (!borderless.isDisposed() &&
-                            
-                        // Make sure that this control is in the active shell, so focus is not stolen from other windows    
-                        (display.getActiveShell() == borderless.getShell()) && 
-                        
-                        // Check that this control still the focus control, despite the recent deactivation.
-                        // BUT... Display.getFocusControl is unreliable when another embedded AWT window has 
-                        // become active, so to be safe, make sure that no other Swing control has been activated
-                        // (otherwise we will steal focus from some other active SwingControl)
-                        (borderless == display.getFocusControl() && (activeBorderless == null))) {
-                        
-                        if (verboseFocusEvents) {
-                            trace("Manually reactivating " + borderless);
-                        }
-                        // Ideally, we would directly activate SwingControl here, but there seems to be 
-                        // no way to do that, so activate the underlying AWT embedded frame
-                        synthesizeWindowActivation(true);
-                        activeBorderless = borderless;
-                    }
+                    activateEmbeddedFrame();
                 }
             });
         }
@@ -453,6 +442,52 @@
     }
 
     
+    // ====
+    
+    /**
+     * Activates the embedded AWT frame, as long as the parent SWT composite has focus and is part of
+     * the active SWT shell. 
+     */
+    public void activateEmbeddedFrame() {
+        assert Display.getCurrent() != null;
+        
+        if (!borderless.isDisposed() &&
+                
+            // Make sure that this control is in the active shell, so focus is not stolen from other windows.
+            // (Note: display.getActiveShell() is not always accurate here, so we use the static instead)
+            (activeShell == borderless.getShell()) && 
+            
+            // Check that this control currently the focus control
+            // BUT... Display.getFocusControl is unreliable when another embedded AWT window has recently 
+            // become active, so to be safe, make sure that no other Swing control has been activated
+            // (otherwise we will steal focus from the other active SwingControl)
+            (borderless == display.getFocusControl() && 
+                    ((activeBorderless == null) || (activeBorderless == borderless)))) {
+            
+            EventQueue.invokeLater(new Runnable() {
+                public void run() {
+                    // Ideally, we would use frame.toFront() here, but that method 
+                    // is a no-op for sun.awt.EmbeddedFrame. The next best thing
+                    // is to request focus on the result of getMostRecentFocusOwner 
+                    // which will preserve any existing focus, or otherwise use the initial
+                    // component.
+                    Component component = frame.getMostRecentFocusOwner();
+                    if (component != null) {
+                        if (verboseFocusEvents) {
+                            trace("Manually activating: " + frame + ", focus component=" + component);
+                        }
+                        component.requestFocus();
+                    } else {
+                        // Nothing can take focus, no point activating the frame. 
+                        if (verboseFocusEvents) {
+                            trace("Ignoring manual activation; no focusable components in " + frame);
+                        }
+                    }
+                }
+            });
+        }
+    }
+
     // ========== Listener implementations
     
 
@@ -461,10 +496,24 @@
         private int currentSwtTraversal = SWT.TRAVERSE_NONE;
 
         public void handleEvent(Event event) {
+            // Track Traversals
             if (event.type == SWT.Traverse) {
                 currentSwtTraversal = event.detail;
+                // No further handling needed
                 return;
             }
+            
+            // Track the currently active shell. This is more reliable than
+            // depending on Display.getActiveShell() which sometimes returns an 
+            // inactive shell. 
+            if ((event.type == SWT.Activate) && (event.widget instanceof Shell)) {
+                activeShell = (Shell)event.widget;
+            }
+            if ((event.type == SWT.Deactivate) && (event.widget instanceof Shell)) {
+                activeShell = null;
+            }
+            
+            // Handle activation of the SWT.EMBEDDED composite. Track the currently active one.  
             if (event.widget == borderless) {
                 switch (event.type) {
                 case SWT.Activate:

Back to the top