diff options
author | Brian de Alwis | 2012-11-13 14:23:09 +0000 |
---|---|---|
committer | Paul Webster | 2012-11-13 18:35:01 +0000 |
commit | 73b7b592c489456dd6ddfc56b8459a4535eb38c7 (patch) | |
tree | 97e33df850e105fb16c1d44383dfdfade8592114 | |
parent | 9aefcba2ae27ba3d1b36baf26009806c0b6dd28d (diff) | |
download | eclipse.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
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> |