Proper selection coloring in JFace Structured Viewers by Christopher-Hermann · Pull Request #3659 · eclipse-platform/eclipse.platform.ui

Christopher-Hermann · 2026-01-12T08:01:03Z

Resumption of #1690

Highlighting problem when using the dark theme on Windows eclipse-platform/eclipse.platform.swt#811

JFace viewer are using OS selection color to highlight the selected item. On some OS this is not accessible. In particular when the eclipse is used with dark theme and the OS is used with light theme, this can cause really bad UI/UX experience. With this change, the selection color can be changed via color preference in the settings of eclipse.

Example
Selection color on Windows Server in dark theme looks really bad:

Testing
On start up of eclipse, the selection colors in column viewers (table and tree viewer) should look like before.
On Windows, the dark theme should predefine the selection colors to fix the bad coloring on some Windows platforms.

In the preferences (Preferences > General > Appearance > Colors and Fonts), there are 4 new properties for the coloring of the viewer selection:

Basic > Viewer selection background (focused)
Basic > Viewer selection background (no focus)
Basic > Viewer selection foreground (focused)
Basic > Viewer selection foreground (no focus)
Changing these colors should affect the coloring of the selection in viewers. The coloring should immediately be applied as soon as the apply button is confirmed. The reset button should reset the coloring to the OS specific highlight coloring, expect for Windows dark theme, where the predefined colors should be restored.

OS Information
The fix is implemented for Windows and Mac only (no GTK).
On GTK the coloring via SWT.EaraseItem is not working properly.

Fixes:
eclipse-platform/eclipse.platform.swt#811
#1688
#1956
#2780
#3570

See:
#1690
#2793

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnViewerSelectionColorListener.java

eclipse-platform-bot · 2026-01-12T08:08:00Z

This pull request changes some projects for the first time in this development cycle.
Therefore the following files need a version increment:

bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF

An additional commit containing all the necessary changes was pushed to the top of this PR's branch. To obtain these changes (for example if you want to push more changes) either fetch from your fork or apply the git patch.

Git patch

From 5392d25fa9fcd2a878559e10634aaca219731dc3 Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <platform-bot@eclipse.org>
Date: Mon, 12 Jan 2026 08:07:38 +0000
Subject: [PATCH] Version bump(s) for 4.39 stream


diff --git a/bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF b/bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF
index a1a0eafd86..f9d231a384 100644
--- a/bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %Plugin.name
 Bundle-SymbolicName: org.eclipse.ui.themes;singleton:=true
-Bundle-Version: 1.2.2900.qualifier
+Bundle-Version: 1.2.3000.qualifier
 Bundle-Vendor: %Plugin.providerName
 Bundle-Localization: plugin
 Require-Bundle: org.eclipse.e4.ui.css.swt.theme
-- 
2.52.0

Further information are available in Common Build Issues - Missing version increments.

github-actions · 2026-01-12T08:08:06Z

Test Results

3 018 files ±0 3 018 suites ±0 2h 13m 12s ⏱️ - 9m 52s
8 229 tests ±0 7 981 ✅ ±0 248 💤 ±0 0 ❌ ±0
23 511 runs ±0 22 720 ✅ ±0 791 💤 ±0 0 ❌ ±0

Results for commit c2654d1. ± Comparison against base commit 8567247.

♻️ This comment has been updated with latest results.

laeubi · 2026-01-12T08:08:16Z

@Christopher-Hermann please use descriptive title and PR description and also adapt the commit message. It is impossible to understand without looking at the code what is actually done here.

Beside that, its always good to show before/after screenshot for visual changes.

Christopher-Hermann · 2026-01-12T08:23:05Z

Added PR title & description. In the current state there are most probably some things left that needs to be cleaned up, tests are missing, etc.

But in the first place I wanted to share the coding with @fedejeanne to get the fix running on all operating systems.
In the last try there were a lot of issue especially for Linux that we need to solve...

fedejeanne · 2026-01-13T13:07:22Z

@Christopher-Hermann thank you for retrying this!

As agreed, I tested on Linux and I found some of the problems that were already mentioned in #1956, namely:

The Viewer selection foreground (no focus) does not apply in the package explorer (MANIFEST.MF) and in the Preferences (Colors and Fonts item)

This situation is worse with the default values since it's then white over white for non-focused elements (MANIFEST.MF in the package explorer)

Icons go missing, both for the focused (orange) and the non-focused (light blue) elements

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnViewerSelectionColorListener.java

...ui.workbench/eclipseui/org/eclipse/ui/internal/themes/ColumnViewerSelectionColorFactory.java

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/StructuredViewer.java

fedejeanne

@Christopher-Hermann I now have some other odd behavior: I see half painted selections :-\

EDIT: I see the icons though, so that's good :-)

Here I changed the Viewer selection background (focused) from orange to green and Viewer selection background (no focus) to yellow and you can see that the selections still have a half-orange part, both the focused and the non-focused ones.

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnViewerSelectionColorListener.java

Christopher-Hermann · 2026-01-28T07:56:29Z

I tried a lot on a Linux VM and followed the Eclipse article where the SWT.EaraseItem is described as: "SWT.EraseItem: allows a client to custom draw a cell’s background and/or selection, and to influence whether the cell’s foreground should be drawn"
But this does not appear to work correctly on Linux (GTK) environments. This leads me to believe there might be a bug in its implementation for this platform.

I’ve implemented a fix/option that influences selection colors, but explicitly only for Windows and macOS, effectively skipping GTK.
I know this is not nice, but according to the amount of issues (all on windows) I guess this solution is better than doing nothing.

Any opinions?

fedejeanne · 2026-02-02T09:26:31Z

@Christopher-Hermann thank you for looking into it and for going into so much trouble for Linux too! I have to say my opinion is somewhat partial since I only develop on Eclipse on Linux in my own free time (we use Windows at work, where my colleagues periodically remind me about this very annoying issue). That being said: I'd be OK to compromise and skip this feature for Linux.

Maybe @akurtakov / @jonahgraham / @iloveeclipse can chip in here since they work on Linux?

@Christopher-Hermann in case your proposal is accepted and you deliver the functionality for Windows and Mac only, you would have to remove the new properties under General > Appearance > Colors and Fonts for Linux though since they don't apply

Christopher-Hermann · 2026-02-06T07:27:32Z

@Christopher-Hermann in case your proposal is accepted and you deliver the functionality for Windows and Mac only, you would have to remove the new properties under General > Appearance > Colors and Fonts for Linux though since they don't apply

I tried to do this, but I didn't find a way to define a color definition only for Windows and Mac, or to hide it on Linux. So if someone has an idea how this could be done please let me know.

Copilot

Pull request overview

This PR adds configurable selection foreground/background colors (focused + no focus) for JFace structured viewers to improve accessibility/contrast, especially in dark theme scenarios on Windows/macOS.

Changes:

Introduces 4 new color definitions exposed in Colors and Fonts preferences for viewer selection coloring.
Adds a JFace EraseItem listener to StructuredViewer to apply the configured selection colors (except on GTK).
Updates dark theme defaults to improve unfocused selection contrast.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 8 comments.

Show a summary per file

File	Description
bundles/org.eclipse.ui/plugin.xml	Adds new color definitions for viewer selection colors backed by a colorFactory.
bundles/org.eclipse.ui/plugin.properties	Adds labels/descriptions for the new color definitions.
bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/themes/ColumnViewerSelectionColorFactory.java	Provides OS-derived default RGBs for the new color definitions.
bundles/org.eclipse.ui.themes/css/dark/e4-dark_preferencestyle.css	Sets dark theme defaults for unfocused viewer selection colors.
bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF	Version bump for the themes bundle.
bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/StructuredViewer.java	Registers the new selection-color listener for structured viewers.
bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnViewerSelectionColorListener.java	Implements selection drawing logic using themed colors and fallbacks.
bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/TableOwnerDrawSupport.java	Uses the new selection drawing helper for owner-drawn tables (content assist).

Comments suppressed due to low confidence (1)

bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/TableOwnerDrawSupport.java:160

In the selected-item path, this method changes the GC colors via drawSelection(event) but never restores oldForeground/oldBackground afterwards. Since SWT reuses the same GC for subsequent painting, this can leak selection colors into later drawing; restore the original GC colors at the end (similar to the non-selected branch).

		// Remember colors to restore the GC later
		Color oldForeground= gc.getForeground();
		Color oldBackground= gc.getBackground();

		if (isSelected) {
			ColumnViewerSelectionColorListener.drawSelection(event);
		} else {
			Color foreground= item.getForeground(index);
			gc.setForeground(foreground);

			Color background= item.getBackground(index);
			gc.setBackground(background);
		}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-02-24T11:47:22Z

...ui.workbench/eclipseui/org/eclipse/ui/internal/themes/ColumnViewerSelectionColorFactory.java

+
+	@Override
+	public RGB createColor() {
+		Display display = Display.getDefault();


Other theme color factories in this package use Display.getCurrent() (and assume UI thread). Using Display.getDefault() here can create/access a Display from the wrong thread and is inconsistent with the established pattern; prefer Display.getCurrent() (with a safe fallback if needed).

Suggested change

Display display = Display.getDefault();

Display display = Display.getCurrent();

if (display == null) {

display = Display.getDefault();

}

Copilot · 2026-02-24T11:47:22Z

...ui.workbench/eclipseui/org/eclipse/ui/internal/themes/ColumnViewerSelectionColorFactory.java

+		if ("SELECTED_CELL_BACKGROUND".equals(color)) { //$NON-NLS-1$
+			return display.getSystemColor(SWT.COLOR_TITLE_BACKGROUND).getRGB();
+
+		} else if ("SELECTED_CELL_FOREGROUND".equals(color)) { //$NON-NLS-1$
+			return display.getSystemColor(SWT.COLOR_WHITE).getRGB();
+
+		} else if ("SELECTED_CELL_BACKGROUND_NO_FOCUS".equals(color)) { //$NON-NLS-1$
+			return display.getSystemColor(SWT.COLOR_TITLE_INACTIVE_BACKGROUND).getRGB();
+
+		} else if ("SELECTED_CELL_FOREGROUND_NO_FOCUS".equals(color)) { //$NON-NLS-1$
+			return display.getSystemColor(SWT.COLOR_TITLE_INACTIVE_FOREGROUND).getRGB();
+


Focused selection foreground is hard-coded to SWT.COLOR_WHITE, while the other defaults are based on title-bar system colors. On platforms/themes where the title background is light, this can produce unreadable selection text; use a matching system foreground (e.g., SWT.COLOR_TITLE_FOREGROUND or SWT.COLOR_LIST_SELECTION_TEXT) instead of forcing white.

Copilot · 2026-02-24T11:47:22Z

bundles/org.eclipse.ui/plugin.properties

+Color.viewerSelectionBackgroundFocused=Viewer selection background (focused)
+Color.viewerSelectionBackgroundFocusedDesc=Background color for selected items in viewers when the control has focus. Defaults to system highlight color. (No effect on Linux)
+Color.viewerSelectionForegroundFocused=Viewer selection foreground (focused)
+Color.viewerSelectionForegroundFocusedDesc=Foreground color for selected items in viewers when the control has focus. (No effect on Linux)
+Color.viewerSelectionBackgroundNoFocus=Viewer selection background (no focus)
+Color.viewerSelectionBackgroundNoFocusDesc=Background color for selected items in viewers when the control does not have focus. (No effect on Linux)
+Color.viewerSelectionForegroundNoFocus=Viewer selection foreground (no focus)
+Color.viewerSelectionForegroundNoFocusDesc=Foreground color for selected items in viewers when the control does not have focus. (No effect on Linux)


The description says this defaults to the “system highlight color”, but the default factory/fallback uses title-bar system colors (e.g., SWT.COLOR_TITLE_BACKGROUND) rather than SWT.COLOR_LIST_SELECTION. Either adjust the implementation to truly default to highlight colors or update the user-facing descriptions to match the actual defaults.

Copilot · 2026-02-24T11:47:23Z

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnViewerSelectionColorListener.java

+ * EraseItem event listener that provides custom selection coloring for JFace
+ * viewers. This listener only activates when no custom owner draw label
+ * provider is registered, ensuring it doesn't conflict with existing custom
+ * drawing implementations.
+ * <p>


The class Javadoc claims the listener “only activates when no custom owner draw label provider is registered”, but the implementation only checks for additional EraseItem listeners and explicitly ignores the OwnerDrawLabelProvider listener. Please update the Javadoc to reflect the actual activation/override rules so API consumers understand when this will run.

Suggested change

* EraseItem event listener that provides custom selection coloring for JFace

* viewers. This listener only activates when no custom owner draw label

* provider is registered, ensuring it doesn't conflict with existing custom

* drawing implementations.

* <p>

* {@code SWT.EraseItem} event listener that provides custom selection coloring

* for JFace viewers.

* <p>

* The listener participates in drawing as long as there are no additional

* custom {@code SWT.EraseItem} listeners installed on the underlying control

* (other than the one used internally by {@link OwnerDrawLabelProvider}).

* This ensures that existing custom drawing implementations which handle

* {@code SWT.EraseItem} themselves can fully control the rendering, including

* selection colors, without interference from this listener.

* </p>

* <p>

Copilot · 2026-02-24T11:47:23Z

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnViewerSelectionColorListener.java

+	/**
+	 * Checks if additional EraseItem listeners were registered after this listener
+	 * that are NOT the OwnerDrawListener. This allows user code to override the
+	 * selection coloring by adding their own EraseItem listener, while still
+	 * allowing StyledCellLabelProvider to work (which uses OwnerDrawListener but
+	 * doesn't draw selection).
+	 *
+	 * @param event the erase event
+	 * @return <code>true</code> if other custom listeners are present that should
+	 *         handle selection, <code>false</code> otherwise
+	 */
+	private boolean hasAdditionalEraseItemListeners(Event event) {
+		if (!(event.widget instanceof Control control)) {
+			return false;
+		}
+
+		Listener[] listeners = control.getListeners(SWT.EraseItem);
+		Object ownerDrawListener = control.getData(OWNER_DRAW_LISTENER_KEY);
+		return Arrays.stream(listeners).anyMatch(l -> l != this && l != ownerDrawListener);
+	}


hasAdditionalEraseItemListeners() documentation says it detects listeners “registered after this listener”, but the implementation doesn’t check registration order—any extra EraseItem listener (even one registered earlier) will disable this selection drawing. Either implement an order-aware check or revise the method comment/behavior to avoid surprising disabling of selection coloring.

Copilot · 2026-02-24T11:47:23Z

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnViewerSelectionColorListener.java

+	/**
+	 * Draws custom selection coloring for the given event.
+	 * <p>
+	 * This method provides consistent selection rendering across different viewers
+	 * and owner draw implementations. It handles both focused and unfocused
+	 * selection states using themed colors from the ColorRegistry with appropriate
+	 * fallbacks.
+	 * </p>
+	 *
+	 * @param event the erase event containing the widget, GC, and coordinates
+	 * @since 3.32
+	 */
+	public static void drawSelection(Event event) {
+		if ("gtk".equals(SWT.getPlatform())) { //$NON-NLS-1$
+			return; // Skip on Linux
+		}
+
+		Control control = (Control) event.widget;
+		GC gc = event.gc;
+
+		Color backgroundColor;
+		Color foregroundColor;
+
+		if (control.isFocusControl()) {
+			backgroundColor = getSelectionColor(COLOR_SELECTION_BG_FOCUS, event.display);
+			foregroundColor = getSelectionColor(COLOR_SELECTION_FG_FOCUS, event.display);
+		} else {
+			backgroundColor = getSelectionColor(COLOR_SELECTION_BG_NO_FOCUS, event.display);
+			foregroundColor = getSelectionColor(COLOR_SELECTION_FG_NO_FOCUS, event.display);
+		}
+
+		gc.setBackground(backgroundColor);
+		gc.setForeground(foregroundColor);
+
+		int width = event.width;
+		if (event.widget instanceof Scrollable scrollable) {
+			width = scrollable.getClientArea().width;
+		}
+
+		gc.fillRectangle(0, event.y, width, event.height);
+
+		// Remove SELECTED and BACKGROUND flags to prevent native drawing from
+		// overwriting our custom colors
+		event.detail &= ~(SWT.SELECTED | SWT.BACKGROUND);
+	}


The drawSelection(Event) Javadoc tags this method as @since 3.32, but the class (and method) are introduced here with @since 3.39. Please correct the @since tag to the version where this API is actually added.

Copilot · 2026-02-24T11:47:23Z

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnViewerSelectionColorListener.java

+		// overwriting our custom colors
+		event.detail &= ~(SWT.SELECTED | SWT.BACKGROUND);


drawSelection(Event) clears SWT.SELECTED and SWT.BACKGROUND in event.detail unconditionally. When this utility is called from SWT.PaintItem (as in TableOwnerDrawSupport), mutating event.detail is unexpected and can interfere with downstream logic that still inspects event.detail during painting; consider only clearing flags when handling SWT.EraseItem (or document this side effect clearly).

Suggested change

// overwriting our custom colors

event.detail &= ~(SWT.SELECTED | SWT.BACKGROUND);

// overwriting our custom colors. This is only relevant for EraseItem

// events; other event types (e.g. PaintItem) may still rely on detail.

if (event.type == SWT.EraseItem) {

event.detail &= ~(SWT.SELECTED | SWT.BACKGROUND);

}

Copilot · 2026-02-24T11:47:24Z

bundles/org.eclipse.jface.text/src/org/eclipse/jface/internal/text/TableOwnerDrawSupport.java

 				break;
 			case SWT.EraseItem:
-				event.detail &= ~SWT.FOREGROUND;
+				event.detail&= ~SWT.FOREGROUND;


This operator formatting (event.detail&= ...) is inconsistent with the surrounding style in this file (spaces around &=). Please keep the existing formatting to avoid noisy diffs and match project conventions.

Suggested change

event.detail&= ~SWT.FOREGROUND;

event.detail &= ~SWT.FOREGROUND;

fedejeanne

I'm looking into this again and trying to find the cause for the misbehavior in Linux, no luck.

I do have some minor comments regarding the writing style though.

fedejeanne · 2026-02-24T10:39:01Z

...ui.workbench/eclipseui/org/eclipse/ui/internal/themes/ColumnViewerSelectionColorFactory.java

+	public RGB createColor() {
+		Display display = Display.getDefault();
+
+		if ("SELECTED_CELL_BACKGROUND".equals(color)) { //$NON-NLS-1$
+			return display.getSystemColor(SWT.COLOR_TITLE_BACKGROUND).getRGB();
+
+		} else if ("SELECTED_CELL_FOREGROUND".equals(color)) { //$NON-NLS-1$
+			return display.getSystemColor(SWT.COLOR_WHITE).getRGB();
+
+		} else if ("SELECTED_CELL_BACKGROUND_NO_FOCUS".equals(color)) { //$NON-NLS-1$
+			return display.getSystemColor(SWT.COLOR_TITLE_INACTIVE_BACKGROUND).getRGB();
+
+		} else if ("SELECTED_CELL_FOREGROUND_NO_FOCUS".equals(color)) { //$NON-NLS-1$
+			return display.getSystemColor(SWT.COLOR_TITLE_INACTIVE_FOREGROUND).getRGB();
+
+		} else {
+			return new RGB(0, 0, 0);
+		}
+	}


How about something more concise like ...

Suggested change

public RGB createColor() {

Display display = Display.getDefault();

if ("SELECTED_CELL_BACKGROUND".equals(color)) { //$NON-NLS-1$

return display.getSystemColor(SWT.COLOR_TITLE_BACKGROUND).getRGB();

} else if ("SELECTED_CELL_FOREGROUND".equals(color)) { //$NON-NLS-1$

return display.getSystemColor(SWT.COLOR_WHITE).getRGB();

} else if ("SELECTED_CELL_BACKGROUND_NO_FOCUS".equals(color)) { //$NON-NLS-1$

return display.getSystemColor(SWT.COLOR_TITLE_INACTIVE_BACKGROUND).getRGB();

} else if ("SELECTED_CELL_FOREGROUND_NO_FOCUS".equals(color)) { //$NON-NLS-1$

return display.getSystemColor(SWT.COLOR_TITLE_INACTIVE_FOREGROUND).getRGB();

} else {

return new RGB(0, 0, 0);

}

}

@Override

public RGB createColor() {

return Display.getDefault().getSystemColor(idToColor(color)).getRGB();

}

private int idToColor(String id) {

return switch (id) {

case "SELECTED_CELL_BACKGROUND" -> SWT.COLOR_TITLE_BACKGROUND; //$NON-NLS-1$

case "SELECTED_CELL_FOREGROUND" -> SWT.COLOR_WHITE; //$NON-NLS-1$

case "SELECTED_CELL_BACKGROUND_NO_FOCUS" -> SWT.COLOR_TITLE_INACTIVE_BACKGROUND; //$NON-NLS-1$

case "SELECTED_CELL_FOREGROUND_NO_FOCUS" -> SWT.COLOR_TITLE_INACTIVE_FOREGROUND; //$NON-NLS-1$

default -> SWT.COLOR_BLACK;

};

}

?

With one change: default (or null) now yield SWT.COLOR_BLACK, which should also work as expected (please double check this).

fedejeanne · 2026-02-24T12:05:00Z

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnViewerSelectionColorListener.java

+		Color backgroundColor;
+		Color foregroundColor;
+
+		if (control.isFocusControl()) {
+			backgroundColor = getSelectionColor(COLOR_SELECTION_BG_FOCUS, event.display);
+			foregroundColor = getSelectionColor(COLOR_SELECTION_FG_FOCUS, event.display);
+		} else {
+			backgroundColor = getSelectionColor(COLOR_SELECTION_BG_NO_FOCUS, event.display);
+			foregroundColor = getSelectionColor(COLOR_SELECTION_FG_NO_FOCUS, event.display);
+		}


Suggested change

Color backgroundColor;

Color foregroundColor;

if (control.isFocusControl()) {

backgroundColor = getSelectionColor(COLOR_SELECTION_BG_FOCUS, event.display);

foregroundColor = getSelectionColor(COLOR_SELECTION_FG_FOCUS, event.display);

} else {

backgroundColor = getSelectionColor(COLOR_SELECTION_BG_NO_FOCUS, event.display);

foregroundColor = getSelectionColor(COLOR_SELECTION_FG_NO_FOCUS, event.display);

}

final Color backgroundColor = getSelectionColor(

control.isFocusControl() ? COLOR_SELECTION_BG_FOCUS : COLOR_SELECTION_BG_NO_FOCUS, event.display);

final Color foregroundColor = getSelectionColor(

control.isFocusControl() ? COLOR_SELECTION_FG_FOCUS : COLOR_SELECTION_FG_NO_FOCUS, event.display);

fedejeanne · 2026-02-24T12:13:28Z

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnViewerSelectionColorListener.java

+	private static Color getSelectionColor(String key, org.eclipse.swt.graphics.Device device) {
+		ColorRegistry registry = JFaceResources.getColorRegistry();
+
+		if (registry.hasValueFor(key)) {
+			return registry.get(key);
+		}
+
+		RGB systemColor;
+		switch (key) {
+		case COLOR_SELECTION_BG_FOCUS:
+			systemColor = device.getSystemColor(SWT.COLOR_TITLE_BACKGROUND).getRGB();
+			break;
+		case COLOR_SELECTION_FG_FOCUS:
+			systemColor = device.getSystemColor(SWT.COLOR_WHITE).getRGB();
+			break;
+		case COLOR_SELECTION_BG_NO_FOCUS:
+			systemColor = device.getSystemColor(SWT.COLOR_TITLE_INACTIVE_BACKGROUND).getRGB();
+			break;
+		case COLOR_SELECTION_FG_NO_FOCUS:
+			systemColor = device.getSystemColor(SWT.COLOR_TITLE_INACTIVE_FOREGROUND).getRGB();
+			break;
+		default:
+			systemColor = device.getSystemColor(SWT.COLOR_LIST_SELECTION).getRGB();
+			break;
+		}
+
+		registry.put(key, systemColor);
+		return registry.get(key);
+	}


Suggested change

private static Color getSelectionColor(String key, org.eclipse.swt.graphics.Device device) {

ColorRegistry registry = JFaceResources.getColorRegistry();

if (registry.hasValueFor(key)) {

return registry.get(key);

}

RGB systemColor;

switch (key) {

case COLOR_SELECTION_BG_FOCUS:

systemColor = device.getSystemColor(SWT.COLOR_TITLE_BACKGROUND).getRGB();

break;

case COLOR_SELECTION_FG_FOCUS:

systemColor = device.getSystemColor(SWT.COLOR_WHITE).getRGB();

break;

case COLOR_SELECTION_BG_NO_FOCUS:

systemColor = device.getSystemColor(SWT.COLOR_TITLE_INACTIVE_BACKGROUND).getRGB();

break;

case COLOR_SELECTION_FG_NO_FOCUS:

systemColor = device.getSystemColor(SWT.COLOR_TITLE_INACTIVE_FOREGROUND).getRGB();

break;

default:

systemColor = device.getSystemColor(SWT.COLOR_LIST_SELECTION).getRGB();

break;

}

registry.put(key, systemColor);

return registry.get(key);

}

private static Color getSelectionColor(String key, org.eclipse.swt.graphics.Device device) {

ColorRegistry registry = JFaceResources.getColorRegistry();

if (!registry.hasValueFor(key)) {

RGB systemColor = device.getSystemColor(idToColor(key)).getRGB();

registry.put(key, systemColor);

}

return registry.get(key);

}

private static int idToColor(String id) {

return switch (id) {

case COLOR_SELECTION_BG_FOCUS -> SWT.COLOR_TITLE_BACKGROUND;

case COLOR_SELECTION_FG_FOCUS -> SWT.COLOR_WHITE;

case COLOR_SELECTION_BG_NO_FOCUS -> SWT.COLOR_TITLE_INACTIVE_BACKGROUND;

case COLOR_SELECTION_FG_NO_FOCUS -> SWT.COLOR_TITLE_INACTIVE_FOREGROUND;

default -> SWT.COLOR_LIST_SELECTION;

};

}

Christopher-Hermann marked this pull request as draft January 12, 2026 08:01

Christopher-Hermann changed the title ~~Selection coloring~~ [WIP] Selection coloring Jan 12, 2026

merks reviewed Jan 12, 2026

View reviewed changes

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnViewerSelectionColorListener.java Outdated Show resolved Hide resolved

Christopher-Hermann changed the title ~~[WIP] Selection coloring~~ [WIP] Proper selection coloring in JFace Structured Viewers Jan 12, 2026

Christopher-Hermann force-pushed the selection_coloring branch 2 times, most recently from db2a1ad to d173442 Compare January 12, 2026 08:35

Copilot AI mentioned this pull request Jan 12, 2026

Add draft PR guidelines to CONTRIBUTING.md laeubi/.github-eclipse-platform#1

Merged

laeubi mentioned this pull request Jan 12, 2026

Add draft PR guidelines to CONTRIBUTING.md eclipse-platform/.github#260

Merged

fedejeanne reviewed Jan 13, 2026

View reviewed changes

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/ColumnViewerSelectionColorListener.java Show resolved Hide resolved

fedejeanne reviewed Jan 13, 2026

View reviewed changes

...ui.workbench/eclipseui/org/eclipse/ui/internal/themes/ColumnViewerSelectionColorFactory.java Outdated Show resolved Hide resolved

fedejeanne reviewed Jan 13, 2026

View reviewed changes

bundles/org.eclipse.jface/src/org/eclipse/jface/viewers/StructuredViewer.java Show resolved Hide resolved

fedejeanne reviewed Jan 15, 2026

View reviewed changes

Andr2ea mentioned this pull request Jan 23, 2026

Windows 11 - Selected Toolbar Icons look light (bad) in the dark theme eclipse-platform/eclipse.platform.swt#217

Open

Christopher-Hermann force-pushed the selection_coloring branch from 606ad8c to 29bf05a Compare January 27, 2026 19:29

eclipse-platform-bot and others added 2 commits January 28, 2026 08:48

Version bump(s) for 4.39 stream

314a045

Handle viewer selection color

c2654d1

Christopher-Hermann force-pushed the selection_coloring branch from 29bf05a to c2654d1 Compare January 28, 2026 07:48

Christopher-Hermann marked this pull request as ready for review January 28, 2026 07:49

Christopher-Hermann changed the title ~~[WIP] Proper selection coloring in JFace Structured Viewers~~ Proper selection coloring in JFace Structured Viewers Jan 28, 2026

fedejeanne requested a review from Copilot February 24, 2026 11:40

Copilot started reviewing on behalf of fedejeanne February 24, 2026 11:41 View session

Copilot AI reviewed Feb 24, 2026

View reviewed changes

fedejeanne reviewed Feb 24, 2026

View reviewed changes

- * EraseItem event listener that provides custom selection coloring for JFace
- * viewers. This listener only activates when no custom owner draw label
- * provider is registered, ensuring it doesn't conflict with existing custom
- * drawing implementations.
- * <p>
+ * {@code SWT.EraseItem} event listener that provides custom selection coloring
+ * for JFace viewers.
+ * <p>
+ * The listener participates in drawing as long as there are no additional
+ * custom {@code SWT.EraseItem} listeners installed on the underlying control
+ * (other than the one used internally by {@link OwnerDrawLabelProvider}).
+ * This ensures that existing custom drawing implementations which handle
+ * {@code SWT.EraseItem} themselves can fully control the rendering, including
+ * selection colors, without interference from this listener.
+ * </p>
+ * <p>

		// overwriting our custom colors
		event.detail &= ~(SWT.SELECTED \| SWT.BACKGROUND);

	event.detail&= ~SWT.FOREGROUND;
	event.detail &= ~SWT.FOREGROUND;

Comments

Conversation

Christopher-Hermann commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

eclipse-platform-bot commented Jan 12, 2026

Uh oh!

github-actions bot commented Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Test Results

Uh oh!

laeubi commented Jan 12, 2026

Uh oh!

Christopher-Hermann commented Jan 12, 2026

Uh oh!

fedejeanne commented Jan 13, 2026

Uh oh!

Uh oh!

Uh oh!

Uh oh!

fedejeanne left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Christopher-Hermann commented Jan 28, 2026

Uh oh!

fedejeanne commented Feb 2, 2026

Uh oh!

Christopher-Hermann commented Feb 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

fedejeanne left a comment

Choose a reason for hiding this comment

Uh oh!

fedejeanne Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

fedejeanne Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

fedejeanne Feb 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Christopher-Hermann commented Jan 12, 2026 •

edited

Loading

github-actions bot commented Jan 12, 2026 •

edited

Loading

fedejeanne left a comment •

edited

Loading