Undo Redo Buttons do not react on E4 handlers

fixes eclipse-platform#977

Undo Redo can now be enabled by E4 Handlers.

How To Test:
Open the E4UndoRedoTestPart using the Quick Access
Check that undo redo is enabled in the menu and tool bar
Check that a pop up appears when undo or redo is pressed

Switch to a text editor, do the same checks but the behavior should match what you expect from the text editor

bundles/org.eclipse.ui.ide/src/org/eclipse/ui/internal/ide/WorkbenchActionBuilder.java

@@ -151,10 +151,6 @@ public class WorkbenchActionBuilder extends ActionBarAdvisor {
 	private IWorkbenchAction forwardHistoryAction;
 
 	// generic retarget actions
-	private IWorkbenchAction undoAction;
-
-	private IWorkbenchAction redoAction;
-
 	private IWorkbenchAction quitAction;
 
 	private IWorkbenchAction goIntoAction;
@@ -409,8 +405,8 @@ protected void fillCoolBar(ICoolBarManager coolBar) {
 		coolBar.add(new GroupMarker(IIDEActionConstants.GROUP_EDIT));
 		IToolBarManager editToolBar = actionBarConfigurer.createToolBarManager();
 		editToolBar.add(new Separator(IWorkbenchActionConstants.EDIT_GROUP));
-		editToolBar.add(undoAction);
-		editToolBar.add(redoAction);
+		editToolBar.add(getUndoItem());
+		editToolBar.add(getRedoItem());
 
 		// Add to the cool bar manager
 		coolBar.add(actionBarConfigurer.createToolBarContributionItem(editToolBar,
@@ -541,8 +537,8 @@ private MenuManager createEditMenu() {
 		MenuManager menu = new MenuManager(IDEWorkbenchMessages.Workbench_edit, IWorkbenchActionConstants.M_EDIT);
 		menu.add(new GroupMarker(IWorkbenchActionConstants.EDIT_START));
 
-		menu.add(undoAction);
-		menu.add(redoAction);
+		menu.add(getUndoItem());
+		menu.add(getRedoItem());
 		menu.add(new GroupMarker(IWorkbenchActionConstants.UNDO_EXT));
 		menu.add(new Separator());
 
@@ -888,8 +884,6 @@ public void dispose() {
 		quickAccessAction = null;
 		backwardHistoryAction = null;
 		forwardHistoryAction = null;
-		undoAction = null;
-		redoAction = null;
 		quitAction = null;
 		goIntoAction = null;
 		backAction = null;
@@ -1013,13 +1007,6 @@ protected void makeActions(final IWorkbenchWindow window) {
 		newWindowAction.setText(IDEWorkbenchMessages.Workbench_openNewWindow);
 		register(newWindowAction);
 
-		undoAction = ActionFactory.UNDO.create(window);
-		register(undoAction);
-
-		redoAction = ActionFactory.REDO.create(window);
-		register(redoAction);
-
-
 		closeAction = ActionFactory.CLOSE.create(window);
 		register(closeAction);
 
@@ -1433,6 +1420,17 @@ private IContributionItem getPinEditorItem() {
 		return ContributionItemFactory.PIN_EDITOR.create(window);
 	}
 
+	private IContributionItem getUndoItem() {
+		return getItem(ActionFactory.UNDO.getId(), ActionFactory.UNDO.getCommandId(), ISharedImages.IMG_TOOL_UNDO,
+				ISharedImages.IMG_TOOL_UNDO_DISABLED, WorkbenchMessages.Workbench_undo,
+				WorkbenchMessages.Workbench_undoToolTip);
+	}
+
+	private IContributionItem getRedoItem() {
+		return getItem(ActionFactory.REDO.getId(), ActionFactory.REDO.getCommandId(), ISharedImages.IMG_TOOL_REDO,
+				ISharedImages.IMG_TOOL_REDO_DISABLED, WorkbenchMessages.Workbench_redo,
+				WorkbenchMessages.Workbench_redoToolTip);
+	}
 	private IContributionItem getCutItem() {
 		return getItem(ActionFactory.CUT.getId(), ActionFactory.CUT.getCommandId(), ISharedImages.IMG_TOOL_CUT,
 				ISharedImages.IMG_TOOL_CUT_DISABLED, WorkbenchMessages.Workbench_cut,

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/api/IActionBarsTest.java

@@ -150,11 +150,13 @@ public void testSetGlobalActionHandler() throws Throwable {
 			MockAction cut = new MockAction();
 			MockAction copy = new MockAction();
 			MockAction undo = new MockAction();
+			MockAction quit = new MockAction();
 
 			// Set actions.
 			bars.setGlobalActionHandler(IWorkbenchActionConstants.CUT, cut);
 			bars.setGlobalActionHandler(IWorkbenchActionConstants.COPY, copy);
 			bars.setGlobalActionHandler(IWorkbenchActionConstants.UNDO, undo);
+			bars.setGlobalActionHandler(IWorkbenchActionConstants.QUIT, quit);
 			bars.updateActionBars();
 
 			// Run the real workbench actions.
@@ -164,37 +166,46 @@ public void testSetGlobalActionHandler() throws Throwable {
 			// anything that has been converted from a RetargetAction in
 			// WorkbenchActionBuilder must be run as a command
 			runMatchingCommand(fWindow, ActionFactory.CUT.getId());
+			runMatchingCommand(fWindow, ActionFactory.UNDO.getId());
 
 			ActionUtil.runActionUsingPath(this, fWindow,
-					IWorkbenchActionConstants.M_EDIT + '/'
-							+ IWorkbenchActionConstants.UNDO);
+					IWorkbenchActionConstants.M_FILE + '/' + IWorkbenchActionConstants.QUIT);
 			assertTrue(cut.hasRun);
 			assertTrue(!copy.hasRun);
 			assertTrue(undo.hasRun);
+			assertTrue(quit.hasRun);
 
 			// Now create a second view and run the actions again.
 			// Our global actions should not be invoked.
-			fPage.showView(MockViewPart.ID2);
-			cut.hasRun = copy.hasRun = undo.hasRun = false;
+			IViewPart part2 = fPage.showView(MockViewPart.ID2);
+
+			// Set again a different MockAction to prevent the actual closing of the IDE
+			// This is necessary as most of available the RetargetActions are converted
+			part2.getViewSite().getActionBars().setGlobalActionHandler(IWorkbenchActionConstants.QUIT,
+					new MockAction());
+
+			cut.hasRun = copy.hasRun = undo.hasRun = quit.hasRun = false;
 			runMatchingCommand(fWindow, ActionFactory.CUT.getId());
+			runMatchingCommand(fWindow, ActionFactory.UNDO.getId());
 			ActionUtil.runActionUsingPath(this, fWindow,
-					IWorkbenchActionConstants.M_EDIT + '/'
-							+ IWorkbenchActionConstants.UNDO);
+					IWorkbenchActionConstants.M_FILE + '/' + IWorkbenchActionConstants.QUIT);
 			assertTrue(!cut.hasRun);
 			assertTrue(!copy.hasRun);
 			assertTrue(!undo.hasRun);
+			assertTrue(!quit.hasRun);
 
 			// Reactivate test view and run actions again.
 			// This time our global actions should be invoked.
 			fPage.activate(part);
-			cut.hasRun = copy.hasRun = undo.hasRun = false;
+			cut.hasRun = copy.hasRun = undo.hasRun = quit.hasRun = false;
 			runMatchingCommand(fWindow, ActionFactory.CUT.getId());
+			runMatchingCommand(fWindow, ActionFactory.UNDO.getId());
 			ActionUtil.runActionUsingPath(this, fWindow,
-					IWorkbenchActionConstants.M_EDIT + '/'
-							+ IWorkbenchActionConstants.UNDO);
+					IWorkbenchActionConstants.M_FILE + '/' + IWorkbenchActionConstants.QUIT);
 			assertTrue(cut.hasRun);
 			assertTrue(!copy.hasRun);
 			assertTrue(undo.hasRun);
+			assertTrue(quit.hasRun);
 		}
 
 	private void runMatchingCommand(IWorkbenchWindow window, String actionId) {

tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/e4/E4UndoRedoTestPart.java

@@ -0,0 +1,48 @@
+
+package org.eclipse.ui.tests.e4;
+
+
+import javax.annotation.PostConstruct;
+import javax.inject.Inject;
+
+import org.eclipse.e4.core.commands.EHandlerService;
+import org.eclipse.e4.core.di.annotations.Execute;
+import org.eclipse.swt.SWT;
+import org.eclipse.swt.widgets.Composite;
+import org.eclipse.swt.widgets.Label;
+import org.eclipse.swt.widgets.MessageBox;
+import org.eclipse.swt.widgets.Shell;
+import org.eclipse.ui.actions.ActionFactory;
+
+public class E4UndoRedoTestPart {
+
+	@Inject
+	private EHandlerService handlerService;
+
+	@PostConstruct
+	public void postConstruct(Composite parent) {
+
+		new Label(parent, SWT.NONE).setText("Press Undo or Redo");
+
+		handlerService.activateHandler(ActionFactory.UNDO.getCommandId(), new Handler("Undo"));
+		handlerService.activateHandler(ActionFactory.REDO.getCommandId(), new Handler("Redo"));
+	}
+
+	private class Handler {
+
+		private String type;
+
+		public Handler(String type) {
+			this.type = type;
+		}
+
+		@Execute
+		public void execute(Shell shell) {
+
+			MessageBox messageBox = new MessageBox(shell, SWT.NONE);
+			messageBox.setMessage(type);
+			messageBox.setText("E4 Undo Redo Test");
+			messageBox.open();
+		}
+	}
+}

tests/org.eclipse.ui.tests/fragment.e4xmi

@@ -1,5 +1,5 @@
 <?xml version="1.0" encoding="ASCII"?>
-<fragment:ModelFragments xmi:version="2.0" xmlns:xmi="http://www.omg.org/XMI" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:basic="http://www.eclipse.org/ui/2010/UIModel/application/ui/basic" xmlns:commands="http://www.eclipse.org/ui/2010/UIModel/application/commands" xmlns:fragment="http://www.eclipse.org/ui/2010/UIModel/fragment" xmi:id="_zPmxwL5fEe2xycAdAs49Bg">
+<fragment:ModelFragments xmi:version="2.0" xmlns:xmi="http://www.omg.org/XMI" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:basic="http://www.eclipse.org/ui/2010/UIModel/application/ui/basic" xmlns:basic_1="http://www.eclipse.org/ui/2010/UIModel/application/descriptor/basic" xmlns:commands="http://www.eclipse.org/ui/2010/UIModel/application/commands" xmlns:fragment="http://www.eclipse.org/ui/2010/UIModel/fragment" xmi:id="_zPmxwL5fEe2xycAdAs49Bg">
   <fragments xsi:type="fragment:StringModelFragment" xmi:id="_2DG0IL5fEe2xycAdAs49Bg" featurename="commands" parentElementId="org.eclipse.e4.legacy.ide.application">
     <elements xsi:type="commands:Command" xmi:id="__OUmgL5fEe2xycAdAs49Bg" elementId="org.eclipse.ui.tests.command.iconTest" commandName="Test Icon Command" commandIconURI="platform:/plugin/org.eclipse.ui.tests/icons/view.gif"/>
     <elements xsi:type="commands:Command" xmi:id="_syckcCn0Ee6GCubbIJXTcg" elementId="org.eclipse.ui.tests.command.openCompositePart" commandName="Open Composite Part"/>
@@ -18,4 +18,7 @@
       </children>
     </elements>
   </fragments>
+  <fragments xsi:type="fragment:StringModelFragment" xmi:id="_FzAhAC05Ee6-nc55BrYmhA" featurename="descriptors" parentElementId="org.eclipse.e4.legacy.ide.application">
+    <elements xsi:type="basic_1:PartDescriptor" xmi:id="_IQpoIC05Ee6-nc55BrYmhA" elementId="org.eclipse.ui.tests.partdescriptor.e4testpart" label="E4UndoRedoTestPart" allowMultiple="true" closeable="true" contributionURI="bundleclass://org.eclipse.ui.tests/org.eclipse.ui.tests.e4.E4UndoRedoTestPart"/>
+  </fragments>
 </fragment:ModelFragments>