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

Bruno Haible wrote:
Actually, when testing the aforementioned scenario on Windows:

  Open the text fields view and the Error Log view, so that the
  Error Log view is visible and the text fields view is hidden. Close Eclipse.
  Re-run Eclipse. Click on the tab header of the text fields view.

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.

I've also done a little refactoring to share more code among the different focus setting methods.

While testing I noticed some intermittent problems on Gtk with focus traversal when two views with embedded Swing components are visible at the same time. I'll look more deeply at these as soon as I get a chance.



### 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.77
diff -u -r1.77 SwingControl.java
--- src/org/eclipse/albireo/core/SwingControl.java	30 Apr 2008 17:49:08 -0000	1.77
+++ src/org/eclipse/albireo/core/SwingControl.java	30 Apr 2008 18:23:00 -0000
@@ -41,6 +41,7 @@
 import org.eclipse.albireo.internal.ComponentDebugging;
 import org.eclipse.albireo.internal.FocusHandler;
 import org.eclipse.albireo.internal.Platform;
+import org.eclipse.albireo.internal.RunnableWithResult;
 import org.eclipse.albireo.internal.SwtPopupHandler;
 import org.eclipse.swt.SWT;
 import org.eclipse.swt.SWTException;
@@ -176,13 +177,36 @@
                      */
                     public boolean forceFocus() {
                         checkWidget();
-                        if ((swingComponent != null) && !swingComponent.isFocusable()) {
-                            return false;
-                        } else {
-                            boolean result = super.forceFocus();
-                            return handleForceFocus(result);
-                        }
+                        return handleFocusOperation(new RunnableWithResult() {
+                            public void run() {
+                                boolean success = superForceFocus();
+                                success = postProcessForceFocus(success);
+                                setResult(new Boolean(success));
+                            }
+                        });
+                    }
+                    /** 
+                     * Overridden to return false and prevent any focus change
+                     * if the embedded Swing component is not focusable. 
+                     */
+                    public boolean setFocus() {
+                        checkWidget();
+                        return handleFocusOperation(new RunnableWithResult() {
+                            public void run() {
+                                boolean success = superSetFocus();
+                                setResult(new Boolean(success));
+                            }
+                        });
+                    }
+                    
+                    private boolean superSetFocus() {
+                        return super.setFocus();
                     }
+                    
+                    private boolean superForceFocus() {
+                        return super.forceFocus();
+                    }
+                    
                 };
         } else {
             // If no border is needed, there is no need to create another
@@ -1246,17 +1270,24 @@
      * <p>
      * 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. 
      */
     public boolean setFocus() {
         checkWidget();
         
-        if ((swingComponent != null) && !swingComponent.isFocusable()) {
-            return false;
+        if (borderlessChild == this) {
+            return handleFocusOperation(new RunnableWithResult() {
+                public void run() {
+                    boolean success = superSetFocus();
+                    setResult(new Boolean(success));
+                }
+            });
         } else {
-            if (borderlessChild == this)
-                return super.setFocus();
-            else
-                return borderlessChild.setFocus();
+            return borderlessChild.setFocus();
         }
     }
 
@@ -1265,33 +1296,84 @@
      * <p>
      * 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. 
      */
     public boolean forceFocus() {
         checkWidget();
 
         if (borderlessChild == this) {
-            if ((swingComponent != null) && !swingComponent.isFocusable()) {
-                return false;
-            } else {
-                boolean result = super.forceFocus();
-                return handleForceFocus(result);
-            }
+            return handleFocusOperation(new RunnableWithResult() {
+                public void run() {
+                    boolean success = superForceFocus();
+                    // Handle the return value
+                    success = postProcessForceFocus(success);
+                    setResult(new Boolean(success));
+                }
+            });
         } else {
-            boolean result = super.forceFocus();
-            return result;
+            return borderlessChild.forceFocus();
         }
+        
     }
 
     /**
      * Postprocess the super.forceFocus() result.
      */
-    boolean handleForceFocus(boolean result) {
+    protected boolean postProcessForceFocus(boolean result) {
         if (focusHandler != null) {
             result = focusHandler.handleForceFocus(result);
         }
         return result;
     }
-
+    
+    /**
+     * Common focus setting/forcing code. Since this may be called for the SwingControl or
+     * a borderless child component, and it may be called for setting or forcing focus, 
+     * the actual code to change focus is passed in a runnable  
+     *  
+     * @param focusSetter - invoked to set or force focus
+     * @return the result of running the focus setter, or true if it was deferred
+     */
+    protected boolean handleFocusOperation(final RunnableWithResult focusSetter) {
+        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();
+            }
+        }
+    }
+    
+    private boolean superSetFocus() {
+        return super.setFocus();
+    }
+    
+    private boolean superForceFocus() {
+        return super.forceFocus();
+    }
+    
     // ============================= Events and Listeners =============================
 
     private List sizeListeners = new ArrayList();
Index: src/org/eclipse/albireo/internal/RunnableWithResult.java
===================================================================
RCS file: src/org/eclipse/albireo/internal/RunnableWithResult.java
diff -N src/org/eclipse/albireo/internal/RunnableWithResult.java
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ src/org/eclipse/albireo/internal/RunnableWithResult.java	1 Jan 1970 00:00:00 -0000
@@ -0,0 +1,16 @@
+package org.eclipse.albireo.internal;
+
+public abstract class RunnableWithResult implements Runnable {
+    private Object result;
+
+    abstract public void run();
+
+    public Object getResult() {
+        return result;
+    }
+    
+    protected void setResult(Object result) {
+        this.result = result;
+    }
+
+}