Bug 302918: ToolItem setDisabledImage isn't used

Modify ToolItem.setDisabledImage() so that it doesn't
call item.setImage(). Override Item.getImage() in ToolItem
to provide more accurate Javadoc, which reflects that
ToolItem.getImage() returns the enabled image only.

Tested on GTK3.24, with Fedora 31. No AllNonBrowser JUnit
tests fail.

Change-Id: I0d8e9b7592c7b934f38521dcf1350112a88f989e
Signed-off-by: Eric Williams <ericwill@redhat.com>

4 files changed, 70 insertions, 17 deletions

diff --git a/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/widgets/ToolItem.java b/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/widgets/ToolItem.java
index b5df4a903b..51c75b221a 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/widgets/ToolItem.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/cocoa/org/eclipse/swt/widgets/ToolItem.java
@@ -637,6 +637,22 @@ public Image getHotImage () {
 	return hotImage;
 }
 
+/**
+ * Returns the receiver's enabled image if it has one, or null
+ * if it does not.
+ *
+ * @return the receiver's enabled image
+ *
+ * @exception SWTException <ul>
+ *    <li>ERROR_WIDGET_DISPOSED - if the receiver has been disposed</li>
+ *    <li>ERROR_THREAD_INVALID_ACCESS - if not called from the thread that created the receiver</li>
+ * </ul>
+ */
+@Override
+public Image getImage () {
+	return super.getImage();
+}
+
 NSString getItemID() {
 	NSString itemID = id;
 
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ToolItem.java b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ToolItem.java
index 9553562596..9bcb423845 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ToolItem.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/gtk/org/eclipse/swt/widgets/ToolItem.java
@@ -45,7 +45,7 @@ public class ToolItem extends Item {
 	long eventHandle, proxyMenuItem, provider;
 	ToolBar parent;
 	Control control;
-	Image hotImage, disabledImage, enabledImage;
+	Image hotImage, disabledImage;
 	String toolTipText;
 	boolean drawHotImage;
 	/** True iff map has been hooked for this ToolItem. See bug 546914. */
@@ -393,6 +393,22 @@ public Image getHotImage () {
 }
 
 /**
+ * Returns the receiver's enabled image if it has one, or null
+ * if it does not.
+ *
+ * @return the receiver's enabled image
+ *
+ * @exception SWTException <ul>
+ *    <li>ERROR_WIDGET_DISPOSED - if the receiver has been disposed</li>
+ *    <li>ERROR_THREAD_INVALID_ACCESS - if not called from the thread that created the receiver</li>
+ * </ul>
+ */
+@Override
+public Image getImage () {
+	return this.image;
+}
+
+/**
  * Returns the receiver's parent, which must be a <code>ToolBar</code>.
  *
  * @return the receiver's parent
@@ -851,7 +867,7 @@ void releaseWidget () {
 	if (parent.currentFocusItem == this) parent.currentFocusItem = null;
 	parent = null;
 	control = null;
-	hotImage = disabledImage = enabledImage = null;
+	hotImage = disabledImage = null;
 	toolTipText = null;
 }
 
@@ -984,16 +1000,8 @@ public void setDisabledImage (Image image) {
 	if ((style & SWT.SEPARATOR) != 0) return;
 	disabledImage = image;
 	if (image != null) {
-		ImageList imageList = parent.imageList;
-		if (imageList == null) imageList = parent.imageList = new ImageList ();
-		int imageIndex = imageList.indexOf (image);
-		if (imageIndex == -1) {
-			imageIndex = imageList.add (image);
-		} else {
-			imageList.put (imageIndex, image);
-		}
 		if (!enabled) {
-			setImage(image);
+			_setImage(image);
 		}
 	}
 }
@@ -1019,8 +1027,8 @@ public void setEnabled (boolean enabled) {
 	long topHandle = topHandle ();
 	if (this.enabled == enabled) return;
 	this.enabled = enabled;
-	if (!enabled && disabledImage != null) setImage (disabledImage);
-	if (enabled && enabledImage != null) setImage (enabledImage);
+	if (!enabled && disabledImage != null) _setImage (disabledImage);
+	if (enabled && this.image != null) _setImage (this.image);
 	GTK.gtk_widget_set_sensitive (topHandle, enabled);
 }
 
@@ -1121,11 +1129,14 @@ public void setImage (Image image) {
 	checkWidget();
 	if ((style & SWT.SEPARATOR) != 0) return;
 	super.setImage (image);
-	if (enabled) enabledImage = image;
 	if (!enabled && disabledImage != image) {
-		enabledImage = image;
 		return;
 	}
+	_setImage(image);
+}
+
+void _setImage (Image image) {
+	if ((style & SWT.SEPARATOR) != 0) return;
 	if (image != null) {
 		ImageList imageList = parent.imageList;
 		if (imageList == null) imageList = parent.imageList = new ImageList ();
diff --git a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolItem.java b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolItem.java
index 2f9ed371a1..34e2926f9b 100644
--- a/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolItem.java
+++ b/bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolItem.java
@@ -331,6 +331,22 @@ public Image getHotImage () {
 }
 
 /**
+ * Returns the receiver's enabled image if it has one, or null
+ * if it does not.
+ *
+ * @return the receiver's enabled image
+ *
+ * @exception SWTException <ul>
+ *    <li>ERROR_WIDGET_DISPOSED - if the receiver has been disposed</li>
+ *    <li>ERROR_THREAD_INVALID_ACCESS - if not called from the thread that created the receiver</li>
+ * </ul>
+ */
+@Override
+public Image getImage () {
+	return super.getImage();
+}
+
+/**
  * Returns the receiver's parent, which must be a <code>ToolBar</code>.
  *
  * @return the receiver's parent
diff --git a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_ToolItem.java b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_ToolItem.java
index 2455082cc1..8418e838b3 100644
--- a/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_ToolItem.java
+++ b/tests/org.eclipse.swt.tests/JUnit Tests/org/eclipse/swt/tests/junit/Test_org_eclipse_swt_widgets_ToolItem.java
@@ -69,7 +69,7 @@ public void test_setDisabledImage() {
 		toolItem.setImage(images[0]);
 		toolItem.setDisabledImage(images[1]);
 		toolItem.setEnabled(false);
-		assertEquals(images[1], toolItem.getImage());
+		assertEquals(images[0], toolItem.getImage());
 
 		toolItem.setEnabled(true);
 		assertEquals(images[0], toolItem.getImage());
@@ -86,7 +86,7 @@ public void test_setDisabledImage() {
 
 		toolItem.setDisabledImage(images[2]);
 		toolItem.setEnabled(false);
-		assertEquals(images[2], toolItem.getImage());
+		assertEquals(images[0], toolItem.getImage());
 
 		toolItem.setEnabled(true);
 		toolItem.setDisabledImage(null);
@@ -100,6 +100,16 @@ public void test_setDisabledImage() {
 		toolItem.setEnabled(false);
 		toolItem.setDisabledImage(null);
 		assertEquals(images[0], toolItem.getImage());
+
+		toolItem.setImage(null);
+		assertEquals(null, toolItem.getImage());
+		toolItem.setEnabled(false);
+		toolItem.setDisabledImage(images[1]);
+		toolItem.setImage(images[1]);
+		assertEquals(images[1], toolItem.getImage());
+		toolItem.setImage(null);
+		assertEquals(null, toolItem.getImage());
+		assertEquals(images[1], toolItem.getDisabledImage());
 	}
 }