Proper selection coloring in JFace Structured Viewers

[Christopher-Hermann]

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

[eclipse-platform-bot]

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.jface.text/META-INF/MANIFEST.MF
bundles/org.eclipse.jface/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 8173e816f6da09758c63518dfb33b1ddc32c3725 Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <platform-bot@eclipse.org>
Date: Fri, 27 Feb 2026 20:32:13 +0000
Subject: [PATCH] Version bump(s) for 4.40 stream


diff --git a/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF b/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF
index 5852190824..74f698f50f 100644
--- a/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.jface.text/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.jface.text
-Bundle-Version: 3.30.0.qualifier
+Bundle-Version: 3.30.100.qualifier
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
 Export-Package: 
diff --git a/bundles/org.eclipse.jface/META-INF/MANIFEST.MF b/bundles/org.eclipse.jface/META-INF/MANIFEST.MF
index 0a2620fd71..a28edbfee3 100644
--- a/bundles/org.eclipse.jface/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.jface/META-INF/MANIFEST.MF
@@ -2,7 +2,7 @@ Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-Name: %pluginName
 Bundle-SymbolicName: org.eclipse.jface;singleton:=true
-Bundle-Version: 3.39.0.qualifier
+Bundle-Version: 3.39.100.qualifier
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
 Export-Package: org.eclipse.jface,
-- 
2.53.0

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

[github-actions]

Test Results

 3 024 files  + 6   3 024 suites  +6   2h 5m 34s ⏱️ - 17m 30s
 8 234 tests + 5   7 985 ✅ + 4  248 💤 ±0  1 ❌ +1 
23 526 runs  +15  22 734 ✅ +14  791 💤 ±0  1 ❌ +1 

For more details on these failures, see this check.

Results for commit 8763def. ± Comparison against base commit 8567247.

This pull request removes 14 and adds 19 tests. Note that renamed tests count towards both.

UIAllTests StartupTestSuite EModelServicePerspectiveFindTest ‑ testAnywhere[0]
UIAllTests StartupTestSuite EModelServicePerspectiveFindTest ‑ testAnywhere[1]
UIAllTests StartupTestSuite EModelServicePerspectiveFindTest ‑ testInActivePerspective[0]
UIAllTests StartupTestSuite EModelServicePerspectiveFindTest ‑ testInActivePerspective[1]
UIAllTests StartupTestSuite EModelServicePerspectiveFindTest ‑ testInAnyPerspective[0]
UIAllTests StartupTestSuite EModelServicePerspectiveFindTest ‑ testInAnyPerspective[1]
UIAllTests StartupTestSuite EModelServicePerspectiveFindTest ‑ testInTrim[0]
UIAllTests StartupTestSuite EModelServicePerspectiveFindTest ‑ testInTrim[1]
UIAllTests StartupTestSuite EModelServicePerspectiveFindTest ‑ testOuterPerspective[0]
UIAllTests StartupTestSuite EModelServicePerspectiveFindTest ‑ testOuterPerspective[1]
…

AllTests AllActionTests ToolBarManagerTest ‑ testSetStyleWhenToolBarDoesNotExist
AllTests FieldAssistTestSuite FieldAssistAPITests ‑ testInitializationWithInvalidCursor
JFaceTextTestSuite CodeMiningTest ‑ testCodeMiningOnZeroWidthCharacterBeforePosition
JFaceTextTestSuite CodeMiningTest ‑ testCodeMiningOnZeroWitdhCharacterAfterPosition
UIAllTests StackRendererTest ‑ testPartReordering
UIAllTests StartupTestSuite EModelServicePerspectiveFindTest Simple ‑ testAnywhere
UIAllTests StartupTestSuite EModelServicePerspectiveFindTest Simple ‑ testInActivePerspective
UIAllTests StartupTestSuite EModelServicePerspectiveFindTest Simple ‑ testInAnyPerspective
UIAllTests StartupTestSuite EModelServicePerspectiveFindTest Simple ‑ testInTrim
UIAllTests StartupTestSuite EModelServicePerspectiveFindTest Simple ‑ testOuterPerspective
…

♻️ This comment has been updated with latest results.

[laeubi]

@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]

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]

@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:

1. 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)

---

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

[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.

[Christopher-Hermann]

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]

@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]

@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]

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();
	}

[fedejeanne]

I'd be fine with using getCurrent() too. WDYT @Christopher-Hermann ?

[Copilot]

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>

[fedejeanne]

@Christopher-Hermann we need to revisit this. Originally, this class was only listening to SWT.EraseItem in the past, which is (partly) why the icons were missing in Linux. It now listens to SWT.EraseItem and SWT.PaintItem and it separates the responsibilities properly i.e. when handling SWT.EraseItem, it merely draws the background (deleting the text and the icons) and when handling SWT.PainItem it draws the icon and the text.

So this raises 2 questions for me:

1. Do we still need to have the logic to avoid installing this listener when isListenerRegistered(control) is true?
2. Is it correct to skip the "erasing" part when there are other listeners that do that? (see usage of hasAdditionalEraseItemListeners(event) in handleEraseItem(event))

WDYT?

[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]

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]

Fixed and also I moved the whole logic to org.eclipse.jface.viewers.ColumnViewerSelectionColorListener. Notice that black is not the fallback anymore, I use SWT.COLOR_LIST_SELECTION instead (which is a system color). Its JavaDoc says

System color used to paint list selection background areas (value is 26).

... which seems to me that it is so arbitrary as new RGB(0, 0, 0) but it doesn't create any new instances.

[Christopher-Hermann]

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.

Thanks for checking it again and for your comments. I did not have time to look into it yet, hopefully I find some time the next days 👍

[fedejeanne]

Aside from the failing checks, this PR is ready to be reviewed.

@Christopher-Hermann I made lots of changes that still need to be squashed. I thought it would be easier for you to review if there are several smaller, more descriptive, changes than only 1 big commit. I tried it on Windows and Linux and it works better than before, at least no icons go missing in Linux.

There is still one thing that doesn't really work as I expect it to: the foreground color in the preferences does not affect views like the package explorer, the outline, (quick) hierarchy view and others. It also doesn't affect lists inside the manifest editor. For what I saw, it only affects the code-completion popup, which is achieved by CompletionProposalDrawSupport (adapted from #2793). Do you have any idea where could we tweak the code so the missing places also honor the foreground color set in the preferences?