435420: [Widgets] Clicking a CCombo pastes the selected element's name into it

https://bugs.eclipse.org/bugs/show_bug.cgi?id=435420

Work around a bug in the responder chain management in the Cocoa implementation of SWT.

4 files changed, 145 insertions, 12 deletions

diff --git a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/AbstractEditor.java b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/AbstractEditor.java
index 43e28c21bb8..0fc09f3b58f 100644
--- a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/AbstractEditor.java
+++ b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/AbstractEditor.java
@@ -10,6 +10,7 @@
  *  Camille Letavernier (CEA LIST) camille.letavernier@cea.fr - Initial API and implementation

  *  Thibault Le Ouay t.leouay@sherpa-eng.com - Add binding implementation

  *  Christian W. Damus (CEA) - bug 402525

+ *  Christian W. Damus (CEA) - bug 435420

  *

  *****************************************************************************/

 package org.eclipse.papyrus.infra.widgets.editors;

@@ -24,15 +25,18 @@ import org.eclipse.core.runtime.IAdaptable;
 import org.eclipse.core.runtime.Platform;

 import org.eclipse.papyrus.infra.widgets.creation.IAtomicOperationExecutor;

 import org.eclipse.swt.SWT;

+import org.eclipse.swt.custom.CCombo;

 import org.eclipse.swt.events.DisposeEvent;

 import org.eclipse.swt.events.DisposeListener;

+import org.eclipse.swt.events.FocusAdapter;

 import org.eclipse.swt.events.FocusEvent;

-import org.eclipse.swt.events.FocusListener;

 import org.eclipse.swt.layout.GridData;

 import org.eclipse.swt.layout.GridLayout;

 import org.eclipse.swt.widgets.Composite;

 import org.eclipse.swt.widgets.Control;

 import org.eclipse.swt.widgets.Label;

+import org.eclipse.swt.widgets.Shell;

+import org.eclipse.swt.widgets.Text;

 import org.eclipse.ui.views.properties.tabbed.TabbedPropertySheetWidgetFactory;

 

 /**

@@ -309,13 +313,7 @@ public abstract class AbstractEditor extends Composite implements DisposeListene
 	 *        to notify the CommitListeners

 	 */

 	protected void setCommitOnFocusLost(Control control) {

-		control.addFocusListener(new FocusListener() {

-

-			@Override

-			public void focusGained(FocusEvent e) {

-				// Nothing

-

-			}

+		control.addFocusListener(new FocusAdapter() {

 

 			@Override

 			public void focusLost(FocusEvent e) {

@@ -419,9 +417,89 @@ public abstract class AbstractEditor extends Composite implements DisposeListene
 	}

 

 	/**

+	 * A hook to call when a control is accepting a focus that is sensitive to glitches in focus management

+	 * on the current SWT platform.

+	 * 

+	 * @see <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=435420">bug 435420</a>

+	 */

+	protected final void acceptingFocus() {

+		// On SWT/Cocoa, veto attempts to give focus to any other control for the current event-loop iteration

+		FocusVeto.vetoFocus(this);

+	}

+	

+	/**

 	 * Queries the model element that I edit.

 	 *

 	 * @return the contextual model element

 	 */

 	protected abstract Object getContextElement();

+	

+	//

+	// Nested types

+	//

+	

+	/**

+	 * A utility that implements a bug in the SWT implementation on Cocoa, in which responder-chain management

+	 * while a {@link CCombo} is trying to accept focus in a Property Sheet that currently does not have focus

+	 * results in the text contents of some unrelated {@link Text} widget being presented in the {@code CCombo'}

+	 * text field.

+	 * 

+	 * @see <a href="https://bugs.eclipse.org/bugs/show_bug.cgi?id=435420">bug 435420</a>

+	 */

+	static class FocusVeto {

+

+		// Only engage this work-around on the Cocoa implementation of SWT because it actually results in

+		// editable CCombos not getting keyboard focus when initially clicked if the Property Sheet is not

+		// yet active

+		private static final boolean IS_SWT_COCOA = Platform.WS_COCOA.equals(Platform.getWS());

+

+		private final Control focusControl;

+

+		private FocusVeto(Control focusControl) {

+			this.focusControl = focusControl;

+			final Shell shell = focusControl.getShell();

+

+			focusControl.getDisplay().asyncExec(new Runnable() {

+

+				@Override

+				public void run() {

+

+					removeFocusVeto(shell, FocusVeto.this);

+				}

+			});

+		}

+

+		Control getFocusControl() {

+			return focusControl;

+		}

+

+		static Control getFocusVetoControl(Control context) {

+			FocusVeto veto = IS_SWT_COCOA ? getFocusVeto(context.getShell()) : null;

+			return (veto == null) ? null : veto.getFocusControl();

+		}

+

+		static void vetoFocus(Control focusControl) {

+			if(IS_SWT_COCOA) {

+				Shell shell = focusControl.getShell();

+				FocusVeto current = getFocusVeto(shell);

+				if(current == null) {

+					setFocusVeto(shell, new FocusVeto(focusControl));

+				}

+			}

+		}

+

+		static FocusVeto getFocusVeto(Shell shell) {

+			return (FocusVeto)shell.getData(FocusVeto.class.getName());

+		}

+

+		static void setFocusVeto(Shell shell, FocusVeto focusVeto) {

+			shell.setData(FocusVeto.class.getName(), focusVeto);

+		}

+

+		static void removeFocusVeto(Shell shell, FocusVeto focusVeto) {

+			if(getFocusVeto(shell) == focusVeto) {

+				shell.setData(FocusVeto.class.getName(), null);

+			}

+		}

+	}

 }

diff --git a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/EditorParentComposite.java b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/EditorParentComposite.java
new file mode 100644
index 00000000000..d4f7388e588
--- /dev/null
+++ b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/EditorParentComposite.java
@@ -0,0 +1,40 @@
+/*
+ * Copyright (c) 2014 CEA and others.
+ * 
+ * All rights reserved. This program and the accompanying materials
+ * are made available under the terms of the Eclipse Public License v1.0
+ * which accompanies this distribution, and is available at
+ * http://www.eclipse.org/legal/epl-v10.html
+ *
+ * Contributors:
+ *   Christian W. Damus (CEA) - Initial API and implementation
+ *
+ */
+package org.eclipse.papyrus.infra.widgets.editors;
+
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Control;
+
+
+/**
+ * The top parent composite for the assembly of tabbed property sheet pages based on the XWT properties model.
+ * Amongst other possible services, this composite works around problems in focus management on some platforms.
+ */
+public class EditorParentComposite extends Composite {
+
+	public EditorParentComposite(Composite parent, int style) {
+		super(parent, style);
+	}
+
+	@Override
+	public boolean setFocus() {
+		Control focusVetoControl = AbstractEditor.FocusVeto.getFocusVetoControl(this);
+
+		if(focusVetoControl != null) {
+			// Don't let me or another control within me take focus
+			return false;
+		}
+
+		return super.setFocus();
+	}
+}
diff --git a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/ReferenceCombo.java b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/ReferenceCombo.java
index 9447822c6e4..5f523bcf3f0 100644
--- a/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/ReferenceCombo.java
+++ b/plugins/infra/widget/org.eclipse.papyrus.infra.widgets/src/org/eclipse/papyrus/infra/widgets/editors/ReferenceCombo.java
@@ -1,5 +1,5 @@
 /*****************************************************************************

- * Copyright (c) 2010 CEA LIST.

+ * Copyright (c) 2010, 2014 CEA LIST and others.

  *

  * All rights reserved. This program and the accompanying materials

  * are made available under the terms of the Eclipse Public License v1.0

@@ -9,6 +9,8 @@
  * Contributors:

  *  Camille Letavernier (CEA LIST) camille.letavernier@cea.fr - Initial API and implementation

  *  Thibault Le Ouay t.leouay@sherpa-eng.com - Add binding implementation

+ *  Christian W. Damus (CEA) - bug 435420

+ *  

  *****************************************************************************/

 package org.eclipse.papyrus.infra.widgets.editors;

 

@@ -30,6 +32,8 @@ import org.eclipse.papyrus.infra.widgets.providers.UnchangedObject;
 import org.eclipse.papyrus.infra.widgets.providers.UnsetObject;

 import org.eclipse.swt.SWT;

 import org.eclipse.swt.custom.CCombo;

+import org.eclipse.swt.events.FocusAdapter;

+import org.eclipse.swt.events.FocusEvent;

 import org.eclipse.swt.graphics.Color;

 import org.eclipse.swt.layout.GridData;

 import org.eclipse.swt.layout.GridLayout;

@@ -109,6 +113,14 @@ public class ReferenceCombo extends AbstractValueEditor { //implements Selection
 		GridData gridData = getDefaultLayoutData();

 		combo.setLayoutData(gridData);

 		gridData.horizontalIndent = FieldDecorationRegistry.getDefault().getMaximumDecorationWidth();

+

+		combo.addFocusListener(new FocusAdapter() {

+

+			@Override

+			public void focusGained(FocusEvent paramFocusEvent) {

+				acceptingFocus();

+			}

+		});

 	}

 

 	/**

@@ -267,7 +279,7 @@ public class ReferenceCombo extends AbstractValueEditor { //implements Selection
 		this.combo = viewer.getCCombo();

 	}

 

-	//FIXME error avec multiplicité nulllpointerexception l285

+	//FIXME error avec multiplicit� nulllpointerexception l285

 	@Override

 	public void updateStatus(IStatus status) {

 		switch(status.getSeverity()) {

diff --git a/plugins/views/properties/org.eclipse.papyrus.views.properties/src/org/eclipse/papyrus/views/properties/xwt/XWTSection.java b/plugins/views/properties/org.eclipse.papyrus.views.properties/src/org/eclipse/papyrus/views/properties/xwt/XWTSection.java
index f70516c3677..074caeaf9e0 100644
--- a/plugins/views/properties/org.eclipse.papyrus.views.properties/src/org/eclipse/papyrus/views/properties/xwt/XWTSection.java
+++ b/plugins/views/properties/org.eclipse.papyrus.views.properties/src/org/eclipse/papyrus/views/properties/xwt/XWTSection.java
@@ -1,5 +1,5 @@
 /*****************************************************************************

- * Copyright (c) 2010 CEA LIST.

+ * Copyright (c) 2010, 2014 CEA LIST and others.

  *

  * All rights reserved. This program and the accompanying materials

  * are made available under the terms of the Eclipse Public License v1.0

@@ -8,6 +8,8 @@
  *

  * Contributors:

  *  Camille Letavernier (CEA LIST) camille.letavernier@cea.fr - Initial API and implementation

+ *  Christian W. Damus (CEA) - bug 435420

+ *  

  *****************************************************************************/

 package org.eclipse.papyrus.views.properties.xwt;

 

@@ -21,6 +23,7 @@ import org.eclipse.jface.viewers.IStructuredSelection;
 import org.eclipse.papyrus.infra.constraints.ConstraintDescriptor;

 import org.eclipse.papyrus.infra.constraints.constraints.Constraint;

 import org.eclipse.papyrus.infra.constraints.runtime.ConstraintFactory;

+import org.eclipse.papyrus.infra.widgets.editors.EditorParentComposite;

 import org.eclipse.papyrus.views.properties.Activator;

 import org.eclipse.papyrus.views.properties.contexts.Section;

 import org.eclipse.papyrus.views.properties.contexts.View;

@@ -74,7 +77,7 @@ public class XWTSection extends AbstractPropertySection implements IChangeListen
 

 	@Override

 	public void createControls(Composite parent, TabbedPropertySheetPage tabbedPropertySheetPage) {

-		self = new Composite(parent, SWT.NONE);

+		self = new EditorParentComposite(parent, SWT.NONE);

 

 		GridLayout layout = new GridLayout(1, false);

 		layout.marginHeight = 0;