Skip to main content
aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorBrian de Alwis2012-11-13 14:23:09 +0000
committerPaul Webster2012-11-13 18:35:01 +0000
commit73b7b592c489456dd6ddfc56b8459a4535eb38c7 (patch)
tree97e33df850e105fb16c1d44383dfdfade8592114
parent9aefcba2ae27ba3d1b36baf26009806c0b6dd28d (diff)
downloadeclipse.platform.ui-73b7b592c489456dd6ddfc56b8459a4535eb38c7.tar.gz
eclipse.platform.ui-73b7b592c489456dd6ddfc56b8459a4535eb38c7.tar.xz
eclipse.platform.ui-73b7b592c489456dd6ddfc56b8459a4535eb38c7.zip
Bug 391232 - ISharedImages constants not recognized in plugin.xmlv20121113-183501
Backport fix from 4.3: * Fix and rename MenuHelper.getIconUrl() to lookup failed icons in the workbench images * Adapt other locations doing ad-hoc icon URI resolution to use MenuHelper.getIconURI(). * Add tests for MenuHelper
-rw-r--r--bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/WorkbenchPage.java15
-rw-r--r--bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/CompatibilityEditor.java17
-rw-r--r--bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/menus/MenuAdditionCacheEntry.java4
-rw-r--r--bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/menus/MenuHelper.java30
-rw-r--r--bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/registry/ViewRegistry.java14
-rw-r--r--tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/MenuHelperTest.java56
-rw-r--r--tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/MenusTestSuite.java1
-rw-r--r--tests/org.eclipse.ui.tests/plugin.xml14
8 files changed, 92 insertions, 59 deletions
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/WorkbenchPage.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/WorkbenchPage.java
index 230e014ec85..2b813cb75d1 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/WorkbenchPage.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/WorkbenchPage.java
@@ -4177,20 +4177,7 @@ public class WorkbenchPage extends CompatibleWorkbenchPage implements
if (descriptor != null) {
IConfigurationElement element = descriptor.getConfigurationElement();
if (element != null) {
- iconURI = element.getAttribute(IWorkbenchRegistryConstants.ATT_ICON);
- if (iconURI != null && !iconURI.startsWith("platform:/plugin/")) { //$NON-NLS-1$
- StringBuilder builder = new StringBuilder("platform:/plugin/"); //$NON-NLS-1$
- builder.append(element.getContributor().getName()).append('/');
-
- // FIXME: need to get rid of $nl$ properly
- // this can be done with FileLocator
- if (iconURI.startsWith("$nl$")) { //$NON-NLS-1$
- iconURI = iconURI.substring(4);
- }
-
- builder.append(iconURI);
- iconURI = builder.toString();
- }
+ iconURI = MenuHelper.getIconURI(element, IWorkbenchRegistryConstants.ATT_ICON);
}
}
return iconURI;
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/CompatibilityEditor.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/CompatibilityEditor.java
index c5455cf8fd4..fd718a73bdd 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/CompatibilityEditor.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/e4/compatibility/CompatibilityEditor.java
@@ -30,6 +30,7 @@ import org.eclipse.ui.internal.EditorReference;
import org.eclipse.ui.internal.PartSite;
import org.eclipse.ui.internal.WorkbenchMessages;
import org.eclipse.ui.internal.WorkbenchPartReference;
+import org.eclipse.ui.internal.menus.MenuHelper;
import org.eclipse.ui.internal.registry.EditorDescriptor;
import org.eclipse.ui.internal.registry.IWorkbenchRegistryConstants;
import org.eclipse.ui.part.AbstractMultiEditor;
@@ -100,20 +101,8 @@ public class CompatibilityEditor extends CompatibilityPart {
if (descriptor != null) {
IConfigurationElement element = descriptor.getConfigurationElement();
if (element != null) {
- String iconURI = element.getAttribute(IWorkbenchRegistryConstants.ATT_ICON);
- if (iconURI != null && !iconURI.startsWith("platform:/plugin/")) { //$NON-NLS-1$
- StringBuilder builder = new StringBuilder("platform:/plugin/"); //$NON-NLS-1$
- builder.append(element.getContributor().getName()).append('/');
-
- // FIXME: need to get rid of $nl$ properly
- // this can be done with FileLocator
- if (iconURI.startsWith("$nl$")) { //$NON-NLS-1$
- iconURI = iconURI.substring(4);
- }
-
- builder.append(iconURI);
- iconURI = builder.toString();
- }
+ String iconURI = MenuHelper.getIconURI(element,
+ IWorkbenchRegistryConstants.ATT_ICON);
part.setIconURI(iconURI);
}
}
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/menus/MenuAdditionCacheEntry.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/menus/MenuAdditionCacheEntry.java
index a410adbb29f..f7c8b98b5d7 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/menus/MenuAdditionCacheEntry.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/menus/MenuAdditionCacheEntry.java
@@ -222,7 +222,7 @@ public class MenuAdditionCacheEntry {
parm.setValue(e.getValue().toString());
item.getParameters().add(parm);
}
- String iconUrl = MenuHelper.getIconUrl(commandAddition,
+ String iconUrl = MenuHelper.getIconURI(commandAddition,
IWorkbenchRegistryConstants.ATT_ICON);
if (iconUrl == null) {
@@ -394,7 +394,7 @@ public class MenuAdditionCacheEntry {
parm.setValue(e.getValue().toString());
item.getParameters().add(parm);
}
- String iconUrl = MenuHelper.getIconUrl(commandAddition,
+ String iconUrl = MenuHelper.getIconURI(commandAddition,
IWorkbenchRegistryConstants.ATT_ICON);
if (iconUrl == null) {
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/menus/MenuHelper.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/menus/MenuHelper.java
index 2a71236f235..eeb92863299 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/menus/MenuHelper.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/menus/MenuHelper.java
@@ -328,27 +328,23 @@ public class MenuHelper {
return element.getAttribute(IWorkbenchRegistryConstants.ATT_TOOLTIP);
}
- static String getIconPath(IConfigurationElement element) {
- return element.getAttribute(IWorkbenchRegistryConstants.ATT_ICON);
- }
-
- static String getDisabledIconPath(IConfigurationElement element) {
- return element.getAttribute(IWorkbenchRegistryConstants.ATT_DISABLEDICON);
- }
-
- static String getHoverIconPath(IConfigurationElement element) {
- return element.getAttribute(IWorkbenchRegistryConstants.ATT_HOVERICON);
- }
-
- static String getIconUrl(IConfigurationElement element, String attr) {
+ public static String getIconURI(IConfigurationElement element, String attr) {
String iconPath = element.getAttribute(attr);
if (iconPath == null) {
return null;
}
+ // If iconPath doesn't specify a scheme, then try to transform to a URL
// RFC 3986: scheme = ALPHA *( ALPHA / DIGIT / "+" / "-" / "." )
// This allows using data:, http:, or other custom URL schemes
if (!iconPath.matches("\\p{Alpha}[\\p{Alnum}+.-]*:.*")) { //$NON-NLS-1$
+ // First attempt to resolve in ISharedImages (e.g. "IMG_OBJ_FOLDER")
+ // as per bug 291232 & AbstractUIPlugin.imageDescriptorFromPlugin().
+ ImageDescriptor d = WorkbenchPlugin.getDefault().getSharedImages()
+ .getImageDescriptor(iconPath);
+ if (d != null) {
+ return getImageUrl(d);
+ }
String extendingPluginId = element.getDeclaringExtension().getContributor().getName();
iconPath = "platform:/plugin/" + extendingPluginId + "/" + iconPath; //$NON-NLS-1$//$NON-NLS-2$
}
@@ -438,7 +434,7 @@ public class MenuHelper {
}
element.setVisibleWhen(getVisibleWhen(menuAddition));
element.setIconURI(MenuHelper
- .getIconUrl(menuAddition, IWorkbenchRegistryConstants.ATT_ICON));
+ .getIconURI(menuAddition, IWorkbenchRegistryConstants.ATT_ICON));
element.setLabel(Util.safeString(text));
return element;
@@ -455,7 +451,7 @@ public class MenuHelper {
text = text.substring(0, idx) + '&' + text.substring(idx);
}
}
- String iconUri = MenuHelper.getIconUrl(element, IWorkbenchRegistryConstants.ATT_ICON);
+ String iconUri = MenuHelper.getIconURI(element, IWorkbenchRegistryConstants.ATT_ICON);
final String cmdId = MenuHelper.getActionSetCommandId(element);
MCommand cmd = ContributionsAnalyzer.getCommandById(app, cmdId);
@@ -538,8 +534,8 @@ public class MenuHelper {
text = text.substring(0, idx) + '&' + text.substring(idx);
}
}
- String iconUri = MenuHelper.getIconUrl(element, IWorkbenchRegistryConstants.ATT_ICON);
- String disabledIconUri = MenuHelper.getIconUrl(element,
+ String iconUri = MenuHelper.getIconURI(element, IWorkbenchRegistryConstants.ATT_ICON);
+ String disabledIconUri = MenuHelper.getIconURI(element,
IWorkbenchRegistryConstants.ATT_DISABLEDICON);
MCommand cmd = ContributionsAnalyzer.getCommandById(app, cmdId);
if (cmd == null) {
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/registry/ViewRegistry.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/registry/ViewRegistry.java
index 63b8009ce9a..22589642ea5 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/registry/ViewRegistry.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/registry/ViewRegistry.java
@@ -121,21 +121,11 @@ public class ViewRegistry implements IViewRegistry {
.getAttribute(IWorkbenchRegistryConstants.ATT_ALLOW_MULTIPLE)));
descriptor.setContributionURI(CompatibilityPart.COMPATIBILITY_VIEW_URI);
- String iconURI = element.getAttribute(IWorkbenchRegistryConstants.ATT_ICON);
+ String iconURI = MenuHelper.getIconURI(element,
+ IWorkbenchRegistryConstants.ATT_ICON);
if (iconURI == null) {
descriptor.setIconURI(MenuHelper.getImageUrl(workbench.getSharedImages()
.getImageDescriptor(ISharedImages.IMG_DEF_VIEW)));
- } else if (!iconURI.startsWith("platform:/plugin/")) { //$NON-NLS-1$
- StringBuilder builder = new StringBuilder("platform:/plugin/"); //$NON-NLS-1$
- builder.append(element.getContributor().getName()).append('/');
-
- // FIXME: need to get rid of $nl$ properly
- if (iconURI.startsWith("$nl$")) { //$NON-NLS-1$
- iconURI = iconURI.substring(4);
- }
-
- builder.append(iconURI);
- descriptor.setIconURI(builder.toString());
} else {
descriptor.setIconURI(iconURI);
}
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/MenuHelperTest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/MenuHelperTest.java
new file mode 100644
index 00000000000..c9658d9aaa0
--- /dev/null
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/MenuHelperTest.java
@@ -0,0 +1,56 @@
+/*******************************************************************************
+ * Copyright (c) 2012 Brian de Alwis 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:
+ * Brian de Alwis - initial API and implementation
+ ******************************************************************************/
+package org.eclipse.ui.tests.menus;
+
+import junit.framework.TestCase;
+
+import org.eclipse.core.runtime.IConfigurationElement;
+import org.eclipse.core.runtime.IExtension;
+import org.eclipse.core.runtime.IExtensionRegistry;
+import org.eclipse.core.runtime.RegistryFactory;
+import org.eclipse.ui.internal.menus.MenuHelper;
+import org.eclipse.ui.internal.registry.IWorkbenchRegistryConstants;
+
+/**
+ * @since 4.2.2
+ */
+public class MenuHelperTest extends TestCase {
+ private IExtensionRegistry registry = RegistryFactory.getRegistry();
+
+ /**
+ * Verify that MenuHelper#getIconURI(IConfigElement,String) looks unresolved
+ * items in the ISharedImages.
+ */
+ public void test391232() {
+ IExtension extension = registry
+ .getExtension("org.eclipse.ui.tests.MenuHelperTest");
+ assertNotNull(extension);
+ assertTrue(extension.getConfigurationElements().length == 1);
+ IConfigurationElement menuContribution = extension
+ .getConfigurationElements()[0];
+ assertEquals(IWorkbenchRegistryConstants.PL_MENU_CONTRIBUTION,
+ menuContribution.getName());
+ assertTrue(menuContribution.getChildren().length > 0);
+ IConfigurationElement menuElement = menuContribution.getChildren()[0];
+ assertEquals(IWorkbenchRegistryConstants.TYPE_MENU,
+ menuElement.getName());
+ assertNotNull(menuElement
+ .getAttribute(IWorkbenchRegistryConstants.ATT_ICON));
+
+ String uri = MenuHelper.getIconURI(menuElement,
+ IWorkbenchRegistryConstants.ATT_ICON);
+ assertNotNull(uri);
+ // contribution specifies "IMG_OBJ_FOLDER"
+ assertEquals(
+ "platform:/plugin/org.eclipse.ui/icons/full/obj16/fldr_obj.gif",
+ uri);
+ }
+}
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/MenusTestSuite.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/MenusTestSuite.java
index ba32aee1c34..128c61642e7 100644
--- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/MenusTestSuite.java
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/menus/MenusTestSuite.java
@@ -40,5 +40,6 @@ public class MenusTestSuite extends TestSuite {
addTest(new TestSuite(Bug231304Test.class));
addTest(new TestSuite(ShowViewMenuTest.class));
addTest(new TestSuite(Bug264804Test.class));
+ addTest(new TestSuite(MenuHelperTest.class));
}
}
diff --git a/tests/org.eclipse.ui.tests/plugin.xml b/tests/org.eclipse.ui.tests/plugin.xml
index 6e2dab28c37..20650ef9fd4 100644
--- a/tests/org.eclipse.ui.tests/plugin.xml
+++ b/tests/org.eclipse.ui.tests/plugin.xml
@@ -4358,4 +4358,18 @@
</run>
</application>
</extension>
+ <extension
+ id="MenuHelperTest"
+ point="org.eclipse.ui.menus">
+ <menuContribution
+ allPopups="false"
+ locationURI="menu:org.eclipse.ui.tests.api.MenuTestHarness">
+ <menu
+ commandId="org.eclipse.ui.edit.copy"
+ icon="IMG_OBJ_FOLDER"
+ id="MenuHelperTest"
+ label="MenuHelperTest">
+ </menu>
+ </menuContribution>
+ </extension>
</plugin>

Back to the top