Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@
import org.eclipse.swt.widgets.Table;
import org.eclipse.swt.widgets.TableItem;

import org.eclipse.jface.viewers.ColumnViewerSelectionColorListener;


/**
* Adds owner draw support for tables.
Expand Down Expand Up @@ -83,7 +85,7 @@ public void handleEvent(Event event) {
measureItem(event);
break;
case SWT.EraseItem:
event.detail &= ~SWT.FOREGROUND;
event.detail&= ~SWT.FOREGROUND;
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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;

Copilot uses AI. Check for mistakes.
break;
case SWT.PaintItem:
performPaint(event);
Expand Down Expand Up @@ -147,7 +149,9 @@ private void performPaint(Event event) {
Color oldForeground= gc.getForeground();
Color oldBackground= gc.getBackground();

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

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,199 @@
/*******************************************************************************
* Copyright (c) 2024 SAP SE.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* SAP SE - initial API and implementation
*******************************************************************************/
package org.eclipse.jface.viewers;

import java.util.Arrays;

import org.eclipse.jface.resource.ColorRegistry;
import org.eclipse.jface.resource.JFaceResources;
import org.eclipse.swt.SWT;
import org.eclipse.swt.graphics.Color;
import org.eclipse.swt.graphics.GC;
import org.eclipse.swt.graphics.RGB;
import org.eclipse.swt.widgets.Control;
import org.eclipse.swt.widgets.Event;
import org.eclipse.swt.widgets.Listener;
import org.eclipse.swt.widgets.Scrollable;

/**
* 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>
Comment on lines +30 to +34
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
* The listener provides different colors for:
* <ul>
* <li>Selected items when the control has focus</li>
* <li>Selected items when the control doesn't have focus</li>
* </ul>
* </p>
*
* @see org.eclipse.jface.viewers.OwnerDrawLabelProvider
* @see org.eclipse.jface.viewers.StyledCellLabelProvider
* @see org.eclipse.jface.viewers.FocusCellOwnerDrawHighlighter
* @since 3.39
*/
public class ColumnViewerSelectionColorListener implements Listener {

private static final String LISTENER_KEY = "org.eclipse.jface.viewers.selection_color_listener"; //$NON-NLS-1$
private static final String OWNER_DRAW_LISTENER_KEY = "owner_draw_label_provider_listener"; //$NON-NLS-1$

private static final String COLOR_SELECTION_BG_FOCUS = "org.eclipse.jface.SELECTION_BACKGROUND_FOCUSED"; //$NON-NLS-1$
private static final String COLOR_SELECTION_FG_FOCUS = "org.eclipse.jface.SELECTION_FOREGROUND_FOCUSED"; //$NON-NLS-1$
private static final String COLOR_SELECTION_BG_NO_FOCUS = "org.eclipse.jface.SELECTION_BACKGROUND_NO_FOCUS"; //$NON-NLS-1$
private static final String COLOR_SELECTION_FG_NO_FOCUS = "org.eclipse.jface.SELECTION_FOREGROUND_NO_FOCUS"; //$NON-NLS-1$

/**
* Registers the selection color listener on the given viewer.
* <p>
* This method is idempotent - calling it multiple times on the same viewer has
* no additional effect.
* </p>
*
* @param viewer the viewer to which the listener should be added
*/
public static void addListenerToViewer(StructuredViewer viewer) {
if ("gtk".equals(SWT.getPlatform())) { //$NON-NLS-1$
return; // Skip on Linux
}

Control control = viewer.getControl();
if (control.isDisposed() || isListenerRegistered(control)) {
return; // Already registered or disposed
}

ColumnViewerSelectionColorListener listener = new ColumnViewerSelectionColorListener();
control.setData(LISTENER_KEY, listener);
control.addListener(SWT.EraseItem, listener);
}

private static boolean isListenerRegistered(Control control) {
return control.getData(LISTENER_KEY) != null;
}

@Override
public void handleEvent(Event event) {
if ((event.detail & SWT.SELECTED) == 0) {
return; // Not selected
}

if (event.widget instanceof Control control && !control.isEnabled()) {
return; // Disabled control
}

if (hasAdditionalEraseItemListeners(event)) {
return; // Let other listeners handle selection
}

drawSelection(event);
}

/**
* 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);
}
Comment on lines +102 to +121
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.

/**
* 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);
}
Comment on lines +143 to +152
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


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);
Comment on lines +165 to +166
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
}
Comment on lines +123 to +167
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.

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);
}
Comment on lines +169 to +197
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


}
Original file line number Diff line number Diff line change
Expand Up @@ -1210,6 +1210,7 @@ public void widgetDefaultSelected(SelectionEvent e) {
});
handler.addPostSelectionListener(widgetSelectedAdapter(this::handlePostSelect));
handler.addOpenListener(StructuredViewer.this::handleOpen);
ColumnViewerSelectionColorListener.addListenerToViewer(this);
}

/**
Expand Down
2 changes: 1 addition & 1 deletion bundles/org.eclipse.ui.themes/META-INF/MANIFEST.MF
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,4 +76,6 @@ IEclipsePreferences#org-eclipse-ui-workbench:org-eclipse-ui-themes { /* pseudo a
'org.eclipse.ui.editors.rangeIndicatorColor=27,118,153'
'org.eclipse.jface.REVISION_NEWEST_COLOR=75,44,3'
'org.eclipse.jface.REVISION_OLDEST_COLOR=154,113,61'
'org.eclipse.jface.SELECTION_FOREGROUND_NO_FOCUS=240,240,240'
'org.eclipse.jface.SELECTION_BACKGROUND_NO_FOCUS=95,95,95'
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
/*******************************************************************************
* Copyright (c) 2024 SAP SE.
*
* This program and the accompanying materials
* are made available under the terms of the Eclipse Public License 2.0
* which accompanies this distribution, and is available at
* https://www.eclipse.org/legal/epl-2.0/
*
* SPDX-License-Identifier: EPL-2.0
*
* Contributors:
* SAP SE - initial API and implementation
*******************************************************************************/
package org.eclipse.ui.internal.themes;

import java.util.Hashtable;
import org.eclipse.core.runtime.IConfigurationElement;
import org.eclipse.core.runtime.IExecutableExtension;
import org.eclipse.swt.SWT;
import org.eclipse.swt.graphics.RGB;
import org.eclipse.swt.widgets.Display;
import org.eclipse.ui.themes.IColorFactory;

/**
* Color factory for viewer selection colors that adapts to the OS/desktop
* theme. Provides default colors based on system colors for focused and
* unfocused selections.
* <p>
* The default colors are based on system title bar colors which automatically
* adapt to light/dark themes and high contrast modes. Themes can override these
* defaults to provide custom styling.
* </p>
*
* @since 3.39
*/
public class ColumnViewerSelectionColorFactory implements IColorFactory, IExecutableExtension {

private String color = null;

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

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.

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

Comment on lines +44 to +55
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.
} else {
return new RGB(0, 0, 0);
}
}
Comment on lines +41 to +59
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


@Override
public void setInitializationData(IConfigurationElement config, String propertyName, Object data) {
if (data instanceof Hashtable table) {
this.color = (String) table.get("color"); //$NON-NLS-1$
}
}
}
9 changes: 9 additions & 0 deletions bundles/org.eclipse.ui/plugin.properties
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,15 @@ Color.revisionNewestDesc=Background color for the newest revision shown in a tex
Color.revisionOldest=Oldest revision background color
Color.revisionOldestDesc=Background color for the oldest revision shown in a text editor's ruler. Together with the newest revision background color this defines a gradient used for all revision from newest to oldest.

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)
Comment on lines +374 to +381
Copy link

Copilot AI Feb 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 uses AI. Check for mistakes.

Color.showKeysForeground=Keys foreground color
Color.showKeysForegroundDesc=Color for foreground of the control to visualize pressed keys
Color.showKeysBackground=Keys background color
Expand Down
Loading
Loading