Description Thomas Gatterweh 2016-10-13 08:46:45 EDT

In an Eclipse-based product, we use numerous toolbar buttons with the idea that the user can configure the specific toolbar item set for his/her needs. This worked fine when using Eclipse 3.8, but when moving to Eclipse 4, user configuration does no longer work.

Steps to reproduce:

1. Create a new plug-in project using the “Hello, World Command” template.

2. Run it. Verify that you see the toolbar button that it adds (Eclipse icon, “Say hello world” text). Open “Customize Perspective” dialog. Verify that this toolbar item appears under “sampleToolbar”, and that you can enable or disable visibility of it.

3. In plugin.xml add a <visibleWhen> clause to the <menuContribution> for the toolbar command, so that it looks like this:
      <menuContribution
            locationURI="toolbar:org.eclipse.ui.main.toolbar?after=additions">
         <toolbar
               id="hello.toolbars.sampleToolbar">
            <command
                  commandId="hello.commands.sampleCommand"
                  icon="icons/sample.gif"
                  tooltip="Say hello world"
                  id="hello.toolbars.sampleCommand">
               <visibleWhen
                     checkEnabled="false">
                  <with
                        variable="activeEditor">
                     <instanceof
                           value="org.eclipse.ui.texteditor.ITextEditor">
                     </instanceof>
                  </with>
               </visibleWhen>
            </command>
         </toolbar>
      </menuContribution>

4. Run it again. Verify that the toolbar item works as expected: When any text editor is active, the toolbar item is shown. When any non-text editor or no editor at all is active, the toolbar item is hidden. 

5. Open “Customize Perspective” dialog. The entry for the toolbar item is shown as before under “sampleToolbar”, however, when clicking on the toolbar item “Say hello world”, an error message is shown: “'Say hello world (Ctrl+6)' command cannot be made visible in this dialog.” When clicking on the toolbar entry itself, the error message is: “'sampleToolbar' cannot be made visible because it is in the unavailable 'null' action set. Would you like to switch to the Action Set Availability tab?” (Neither Yes nor No takes you anywhere in this case, btw).

Expected: It should be possible to configure visibility of the toolbar item, same as in step 2. The toolbar item should be visible when it is set visible in the Customize Perspective dialog and when the <visibleWhen> clause evaluates to true.

The current behavior is a regression in Eclipse 4. The same works as expected in Eclipse 3.8.

I did some debugging, and what I found:

The Customize Perspective dialog allows clicking on a toolbar item only when that item is “available”: UnavailableContributionItemCheckListener.checkStateChanged(CheckStateChangedEvent) tests CustomizePerspectiveDialog.isEffectivelyAvailable(DisplayItem, ViewerFilter), and this one uses CustomizePerspectiveDialog.isAvailable(DisplayItem). This isAvailable test returns false because in its final line it tests item.getIContributionItem().isVisible(), that returns false here because the contribution item is a (temporary) copy and the copy code in org.eclipse.e4.ui.workbench.renderers.swt.ToolBarContributionRecord.mergeIntoModel() contains these lines that look for <visibleWhen> clauses:

	// if a visibleWhen clause is defined, the item should not be
	// visible until the clause has been evaluated and returned 'true'
	copy.setVisible(!anyVisibleWhen());

This ultimately disables that the visibility of this item can be configured in the Customize Perspective dialog.

IMHO, the problem is that CustomizePerspectiveDialog.isAvailable incorporates the contribution item’s isVisible() at all. Beside the fact that it is not correctly evaluated in this case, but assuming that it would correctly represent the result of the <visibleWhen> clause, I don’t think it is user friendly by the dialog to show elements that are sometimes changeable, sometimes not, at the discretion of a clause of which the user most likely is not aware at all.

My suggested fix is to remove that test, and allow the item to be “available” if a contribution item exists, regardless if that’s current visibility evaluates to true or false:

CustomizePerspectiveDialog.java line 1386:

	static boolean isAvailable(DisplayItem item) {
		if (item.getActionSet() != null && item.getChildren().isEmpty()) {
			return item.getActionSet().isActive();
		}
		for (TreeItem treeItem : item.getChildren()) {
			DisplayItem child = (DisplayItem) treeItem;
			if (isAvailable(child)) {
				return true;
			}
		}
-		return item.getIContributionItem() != null && item.getIContributionItem().isVisible();
+		return item.getIContributionItem() != null;
	}
 
With this change, everything works fine, and for a quick check there are no significant changes for other toolbar items in the Customize Perspective dialog.