[Win32] Keep tool bar normal, hot and disabled image lists index-aligned

[HeikoKlare]

Problem

On Windows a ToolItem could render another item's hot (hover) or disabled icon — no multiple monitors and no image disposal required. Setting an image, then a hot image, then clearing the image while adding a new item to the tool bar is already enough to trigger it.

A tool bar keeps three native image lists (normal, hot, disabled), but each button stores a single image index that addresses all three lists at once. The three lists therefore have to stay index-aligned. Two code paths in ToolItem.updateImages broke that invariant:

• Clearing the normal image while a hot image is still set freed the normal and disabled slots but kept the hot slot occupied. A later item that reused the freed normal slot then rendered the first item's stale hot icon on hover.
• Adding a first image appended it to each list with three independent add() calls. Once the lists' free-slot patterns diverged (as caused by the first defect), those calls could return different indices. Only the normal list's index is written to the button, so its slot could end up pointing at another item's hot or disabled image.

Contributes to #3466.

Fix

Enforce one index per item across all three lists:

• Free the hot slot too when the normal image is cleared, so the hot list's free-slot pattern no longer diverges from the normal and disabled lists.
• Derive the slot index once from the normal list and write the hot and disabled lists at that exact index (instead of relying on three independent add() calls returning matching indices). ToolBar.updateOrientation adopts the same discipline.

This is implemented with a small ImageList.putAt(index, image) that stores an image at a caller-chosen index (appending when the index equals the current size, otherwise replacing or, for a null image, clearing the slot). add(image) is now implemented on top of it by first computing the target index.

Reproduction

Run the snippet below and hover the mouse over the second (blue) button:

• Before the fix: the blue button turns green — it renders the first item's leftover hot image, even though it has no hot image of its own.
• After the fix: the blue button stays blue.

Before (wrong behavior):

After (corrected behavior):

/*
 * Reproduction for a Win32 tool bar item rendering another item's hot (hover)
 * icon, without any multiple monitors or image disposal involved.
 *
 * Steps performed below:
 *   - item0 gets a RED normal image and a GREEN hot image
 *   - item0's image is cleared (setImage(null)) while the hot image stays set
 *   - item1 gets a BLUE normal image and NO hot image
 *
 * Hover the mouse over the second (blue) button:
 *   Expected (fixed): it stays BLUE (item1 has no hot image).
 *   Bug:              it turns GREEN, i.e. item1 renders item0's leftover hot image.
 */
import org.eclipse.swt.SWT;
import org.eclipse.swt.graphics.Color;
import org.eclipse.swt.graphics.GC;
import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.graphics.RGB;
import org.eclipse.swt.layout.GridLayout;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Label;
import org.eclipse.swt.widgets.Shell;
import org.eclipse.swt.widgets.ToolBar;
import org.eclipse.swt.widgets.ToolItem;

public class ToolItemHotImageReproduction {

	public static void main(String[] args) {
		Display display = new Display();
		Image red = solidImage(display, new RGB(220, 40, 40));
		Image green = solidImage(display, new RGB(40, 180, 40));
		Image blue = solidImage(display, new RGB(40, 40, 220));

		Shell shell = new Shell(display);
		shell.setLayout(new GridLayout());
		shell.setText("ToolItem hot image reproduction");

		ToolBar bar = new ToolBar(shell, SWT.FLAT);

		ToolItem item0 = new ToolItem(bar, SWT.PUSH);
		item0.setImage(red);
		item0.setHotImage(green); // item0 shows GREEN on hover
		item0.setImage(null);     // clear item0's image; its hot image stays set

		ToolItem item1 = new ToolItem(bar, SWT.PUSH);
		item1.setImage(blue);     // item1: BLUE, no hot image

		Label hint = new Label(shell, SWT.WRAP);
		hint.setText("Hover the mouse over the second (blue) button.\n"
				+ "Expected: it stays BLUE (it has no hot image).\n"
				+ "Bug: it turns GREEN (item0's leftover hot image).");

		shell.setSize(360, 150);
		shell.open();
		while (!shell.isDisposed()) {
			if (!display.readAndDispatch()) display.sleep();
		}

		red.dispose();
		green.dispose();
		blue.dispose();
		display.dispose();
	}

	private static Image solidImage(Display display, RGB rgb) {
		Image image = new Image(display, 16, 16);
		GC gc = new GC(image);
		Color color = new Color(display, rgb);
		gc.setBackground(color);
		gc.fillRectangle(image.getBounds());
		gc.dispose();
		return image;
	}
}

[github-actions]

Test Results (win32)

   33 files  ±0     33 suites  ±0   5m 12s ⏱️ -4s
4 861 tests ±0  4 785 ✅ ±0  76 💤 ±0  0 ❌ ±0 
1 390 runs  ±0  1 366 ✅ ±0  24 💤 ±0  0 ❌ ±0 

Results for commit 6941171. ± Comparison against base commit 7b3acfd.

♻️ This comment has been updated with latest results.

[Copilot]

Pull request overview

This PR fixes a Win32-specific toolbar rendering defect where a ToolItem could show another item’s hot (hover) or disabled image due to the native image lists (normal/hot/disabled) becoming index-misaligned. It enforces a single authoritative slot index per item across all three image lists to prevent stale or cross-item icon lookups.

Changes:

• ToolItem.updateImages: derive the slot index once from the normal image list and write hot/disabled images at that same index; additionally clear the hot slot when the normal image is cleared.
• ToolBar.updateOrientation: when rebuilding image lists, keep new hot/disabled lists aligned by writing at the normal list’s returned index (instead of independent add(...) calls).
• ImageList: introduce putAt(index, image) and implement add(image) on top of it.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolItem.java	Keeps normal/hot/disabled image list indices aligned and clears hot slots when the normal image is cleared.
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/widgets/ToolBar.java	Preserves index alignment across rebuilt image lists during orientation updates.
bundles/org.eclipse.swt/Eclipse SWT/win32/org/eclipse/swt/internal/ImageList.java	Adds putAt(...) to support caller-chosen indices and refactors add(...) to use it.

[r-mennig]

ImageList now contains both a public putAt(int index, Image image) and a public put(int index, Image image) method where putAt allows appending and put does not. So maybe putOrAppend would be a better name for this method?