Attachment 166814 Details for Bug 310601 – patch

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

[patch] patch

bug301196.txt (text/plain), 12.07 KB, created by Susan McCourt

on 2010-05-03 13:59:38 EDT

(hide)

Description:

Filename:

MIME Type:

Creator: Susan McCourt

Created: 2010-05-03 13:59:38 EDT

Size: 12.07 KB

patch

obsolete

>### Eclipse Workspace Patch 1.0
>#P org.eclipse.ui.tests
>Index: Eclipse UI Tests/org/eclipse/ui/tests/fieldassist/AbstractContentAssistCommandAdapterTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/fieldassist/AbstractContentAssistCommandAdapterTest.java,v
>retrieving revision 1.3
>diff -u -r1.3 AbstractContentAssistCommandAdapterTest.java
>--- Eclipse UI Tests/org/eclipse/ui/tests/fieldassist/AbstractContentAssistCommandAdapterTest.java	14 Oct 2009 22:36:00 -0000	1.3
>+++ Eclipse UI Tests/org/eclipse/ui/tests/fieldassist/AbstractContentAssistCommandAdapterTest.java	15 Apr 2010 00:16:57 -0000
>@@ -29,7 +29,7 @@
> 				IWorkbenchCommandConstants.EDIT_CONTENT_ASSIST, null);
> 	}
> 	
>-	public void testHandlerPromptsPopup() throws Exception {
>+	public void disabletestHandlerPromptsPopup() throws Exception {
> 		getFieldAssistWindow().open();
> 
> 		sendFocusInToControl();
>@@ -51,7 +51,7 @@
> 	 * <li>The list shows up (the bug was reporting that the list disappeared)</li>
> 	 * </ol>
> 	 */
>-	public void testBug271339EmptyAutoActivationCharacters() throws Exception {
>+	public void disabletestBug271339EmptyAutoActivationCharacters() throws Exception {
> 		getFieldAssistWindow().open();
> 
> 		sendFocusInToControl();
>@@ -74,7 +74,7 @@
> 	 * autoactivation characters have been set</li>
> 	 * </ol>
> 	 */
>-	public void testBug271339EmptyAutoActivationCharacters2() throws Exception {
>+	public void disabletestBug271339EmptyAutoActivationCharacters2() throws Exception {
> 		getFieldAssistWindow().open();
> 
> 		sendFocusInToControl();
>@@ -97,7 +97,7 @@
> 	 * <li>the popup should remain open</li>
> 	 * </ol>
> 	 */
>-	public void testBug271339EmptyAutoActivationCharacters3() throws Exception {
>+	public void disabletestBug271339EmptyAutoActivationCharacters3() throws Exception {
> 		getFieldAssistWindow().open();
> 
> 		sendFocusInToControl();
>Index: Eclipse UI Tests/org/eclipse/ui/tests/fieldassist/FieldAssistTestSuite.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/fieldassist/FieldAssistTestSuite.java,v
>retrieving revision 1.3
>diff -u -r1.3 FieldAssistTestSuite.java
>--- Eclipse UI Tests/org/eclipse/ui/tests/fieldassist/FieldAssistTestSuite.java	17 Oct 2009 23:20:16 -0000	1.3
>+++ Eclipse UI Tests/org/eclipse/ui/tests/fieldassist/FieldAssistTestSuite.java	15 Apr 2010 00:16:57 -0000
>@@ -30,8 +30,7 @@
> 	 */
> 	public FieldAssistTestSuite() {
> 		addTest(new TestSuite(FieldAssistAPITest.class));
>-		// temporarily disabling tests, see bug 275393
>-		// addTest(new TestSuite(ComboContentAssistCommandAdapterTest.class));
>-		// addTest(new TestSuite(TextContentAssistCommandAdapterTest.class));
>+		addTest(new TestSuite(ComboContentAssistCommandAdapterTest.class));
>+		addTest(new TestSuite(TextContentAssistCommandAdapterTest.class));
> 	}
> }
>Index: Eclipse UI Tests/org/eclipse/ui/tests/fieldassist/TextContentAssistCommandAdapterTest.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/fieldassist/TextContentAssistCommandAdapterTest.java,v
>retrieving revision 1.1
>diff -u -r1.1 TextContentAssistCommandAdapterTest.java
>--- Eclipse UI Tests/org/eclipse/ui/tests/fieldassist/TextContentAssistCommandAdapterTest.java	14 Oct 2009 18:00:43 -0000	1.1
>+++ Eclipse UI Tests/org/eclipse/ui/tests/fieldassist/TextContentAssistCommandAdapterTest.java	15 Apr 2010 00:16:57 -0000
>@@ -11,7 +11,18 @@
>  ******************************************************************************/
> package org.eclipse.ui.tests.fieldassist;
> 
>+import org.eclipse.core.commands.AbstractHandler;
>+import org.eclipse.core.commands.ExecutionEvent;
>+import org.eclipse.core.expressions.EvaluationResult;
>+import org.eclipse.core.expressions.Expression;
>+import org.eclipse.core.expressions.ExpressionInfo;
>+import org.eclipse.core.expressions.IEvaluationContext;
> import org.eclipse.jface.tests.fieldassist.AbstractFieldAssistWindow;
>+import org.eclipse.ui.ISources;
>+import org.eclipse.ui.IWorkbenchCommandConstants;
>+import org.eclipse.ui.PlatformUI;
>+import org.eclipse.ui.handlers.IHandlerActivation;
>+import org.eclipse.ui.handlers.IHandlerService;
> 
> public class TextContentAssistCommandAdapterTest extends
> 		AbstractContentAssistCommandAdapterTest {
>@@ -20,4 +31,39 @@
> 		return new TextCommandFieldAssistWindow();
> 	}
> 
>+	/**
>+	 * bug 301196: [FieldAssist] ContentAssistCommandAdapter should provide an activation expression when activating the command handler
>+	 * https://bugs.eclipse.org/bugs/show_bug.cgi?id=301196
>+	 * 
>+	 */
>+	public void testBug301196CorrectHandlerConflictResolution() throws Exception {
>+		
>+		final boolean[] handlerInvocationIndicator = new boolean[] {false};
>+		
>+		IHandlerService service = (IHandlerService) PlatformUI.getWorkbench().getService(IHandlerService.class);
>+		IHandlerActivation handlerActivation = service.activateHandler(IWorkbenchCommandConstants.EDIT_CONTENT_ASSIST, new AbstractHandler() {
>+			
>+			public Object execute(ExecutionEvent event) {
>+				handlerInvocationIndicator[0] = true;
>+				return null;
>+			}
>+		},new Expression() {
>+			public void collectExpressionInfo(ExpressionInfo info) {
>+				info.addVariableNameAccess(ISources.ACTIVE_SHELL_NAME);
>+			}
>+			public EvaluationResult evaluate(IEvaluationContext context) {
>+				return EvaluationResult.TRUE; // always enabled
>+			}
>+		});
>+		try {
>+			getFieldAssistWindow().open();
>+			sendFocusInToControl();
>+			executeContentAssistHandler();
>+			// assertTwoShellsUp();
>+			// The handler registered by the command adapter should win over the test handler.
>+			assertFalse("test handler should not have been activated", handlerInvocationIndicator[0]);
>+		} finally {
>+			service.deactivateHandler(handlerActivation);
>+		}
>+	}
> }
>#P org.eclipse.ui.workbench
>Index: Eclipse UI/org/eclipse/ui/fieldassist/ContentAssistCommandAdapter.java
>===================================================================
>RCS file: /cvsroot/eclipse/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/fieldassist/ContentAssistCommandAdapter.java,v
>retrieving revision 1.14
>diff -u -r1.14 ContentAssistCommandAdapter.java
>--- Eclipse UI/org/eclipse/ui/fieldassist/ContentAssistCommandAdapter.java	13 Oct 2009 19:04:19 -0000	1.14
>+++ Eclipse UI/org/eclipse/ui/fieldassist/ContentAssistCommandAdapter.java	15 Apr 2010 00:16:59 -0000
>@@ -7,6 +7,7 @@
>  *
>  * Contributors:
>  *     IBM Corporation - initial API and implementation
>+ *     David Green - fix for bug 301196: [FieldAssist] ContentAssistCommandAdapter should provide an activation expression when activating the command handler
>  *******************************************************************************/
> 
> package org.eclipse.ui.fieldassist;
>@@ -14,6 +15,10 @@
> import org.eclipse.core.commands.AbstractHandler;
> import org.eclipse.core.commands.ExecutionEvent;
> import org.eclipse.core.commands.IHandler;
>+import org.eclipse.core.expressions.EvaluationResult;
>+import org.eclipse.core.expressions.Expression;
>+import org.eclipse.core.expressions.ExpressionInfo;
>+import org.eclipse.core.expressions.IEvaluationContext;
> import org.eclipse.jface.fieldassist.ContentProposalAdapter;
> import org.eclipse.jface.fieldassist.ControlDecoration;
> import org.eclipse.jface.fieldassist.FieldDecoration;
>@@ -24,15 +29,15 @@
> import org.eclipse.swt.SWT;
> import org.eclipse.swt.events.DisposeEvent;
> import org.eclipse.swt.events.DisposeListener;
>-import org.eclipse.swt.events.FocusEvent;
>-import org.eclipse.swt.events.FocusListener;
> import org.eclipse.swt.widgets.Control;
>+import org.eclipse.ui.ISources;
> import org.eclipse.ui.IWorkbenchCommandConstants;
> import org.eclipse.ui.PlatformUI;
> import org.eclipse.ui.handlers.IHandlerActivation;
> import org.eclipse.ui.handlers.IHandlerService;
> import org.eclipse.ui.internal.WorkbenchMessages;
> import org.eclipse.ui.keys.IBindingService;
>+import org.eclipse.ui.swt.IFocusService;
> 
> /**
>  * ContentAssistCommandAdapter extends {@link ContentProposalAdapter} to invoke
>@@ -51,7 +56,14 @@
>  */
> public class ContentAssistCommandAdapter extends ContentProposalAdapter {
> 
>+	// ID used in the decoration registry.
> 	private static final String CONTENT_ASSIST_DECORATION_ID = "org.eclipse.ui.fieldAssist.ContentAssistField"; //$NON-NLS-1$
>+
>+	// ID used when registering our control with the focus service. Since we
>+	// never test the control ID in our activation expression, we can safely use
>+	// the same ID for all controls
>+	private static final String CONTROL_ID = "org.eclipse.ui.fieldAssist.ContentAssistField.ControlID"; //$NON-NLS-1$
>+
> 	private String commandId;
> 
> 	/**
>@@ -67,10 +79,6 @@
> 	// a platform UI preference.
> 	private static final int DEFAULT_AUTO_ACTIVATION_DELAY = 500;
> 
>-	private IHandlerService handlerService;
>-
>-	private IHandlerActivation activeHandler;
>-
> 	private IHandler proposalHandler = new AbstractHandler() {
> 		public Object execute(ExecutionEvent event) {
> 			openProposalPopup();
>@@ -168,16 +176,8 @@
> 		// Set a default autoactivation delay.
> 		setAutoActivationDelay(DEFAULT_AUTO_ACTIVATION_DELAY);
> 
>-		// Cache the handler service so we don't have to retrieve it each time
>-		this.handlerService = (IHandlerService) PlatformUI.getWorkbench()
>-				.getService(IHandlerService.class);
>-
>-		// Add listeners to the control to manage activation of the handler
>-		addListeners(control);
>-
>-		if (control.isFocusControl()) {
>-			activateHandler();
>-		}
>+		// Activate the handler
>+		activateHandler(control);
> 
> 		if (installDecoration) {
> 			// Note top left is used for compatibility with 3.2, although
>@@ -191,31 +191,6 @@
> 
> 	}
> 
>-	/*
>-	 * Add the listeners needed in order to activate the content assist command
>-	 * on the control.
>-	 */
>-	private void addListeners(Control control) {
>-		control.addFocusListener(new FocusListener() {
>-			public void focusLost(FocusEvent e) {
>-				deactivateHandler();
>-			}
>-
>-			public void focusGained(FocusEvent e) {
>-				if (isEnabled()) {
>-					activateHandler();
>-				} else {
>-					deactivateHandler();
>-				}
>-			}
>-		});
>-		control.addDisposeListener(new DisposeListener() {
>-			public void widgetDisposed(DisposeEvent e) {
>-				deactivateHandler();
>-			}
>-		});
>-	}
>-
> 	/**
> 	 * Return the string command ID of the command used to invoke content
> 	 * assist.
>@@ -283,26 +258,33 @@
> 				decoration.hide();
> 			}
> 		}
>-		if (getControl().isFocusControl()) {
>-			if (enabled) {
>-				activateHandler();
>-			} else {
>-				deactivateHandler();
>-			}
>-		}
>-	}
>-
>-	private void activateHandler() {
>-		if (activeHandler == null) {
>-			activeHandler = handlerService.activateHandler(commandId,
>-					proposalHandler);
>-		}
> 	}
> 
>-	private void deactivateHandler() {
>-		if (activeHandler != null) {
>-			handlerService.deactivateHandler(activeHandler);
>-			activeHandler = null;
>+	private void activateHandler(final Control control) {
>+		IFocusService fs = (IFocusService) PlatformUI.getWorkbench()
>+				.getService(IFocusService.class);
>+		final IHandlerService hs = (IHandlerService) PlatformUI.getWorkbench().getService(
>+				IHandlerService.class);
>+		if (fs != null && hs != null) {
>+			fs.addFocusTracker(control, CONTROL_ID);
>+			final IHandlerActivation handlerActivation = hs.activateHandler(commandId,
>+					proposalHandler, new Expression() {
>+						public EvaluationResult evaluate(IEvaluationContext context) {
>+							return context.getVariable(ISources.ACTIVE_FOCUS_CONTROL_NAME) == control
>+									&& ContentAssistCommandAdapter.this.isEnabled() ? EvaluationResult.TRUE
>+									: EvaluationResult.FALSE;
>+						}
>+
>+						public void collectExpressionInfo(final ExpressionInfo info) {
>+							info.addVariableNameAccess(ISources.ACTIVE_FOCUS_CONTROL_NAME);
>+						}
>+
>+				});
>+			control.addDisposeListener(new DisposeListener() {
>+				public void widgetDisposed(DisposeEvent e) {
>+					hs.deactivateHandler(handlerActivation);
>+				}
>+			});
> 		}
> 	}
> }

Actions: View | Diff

Attachments on bug 310601: 166814 | 166818