From b8638763ed68e270cbcb3be2d73962a64f584379 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Fri, 4 Sep 2026 15:34:03 +0200 Subject: [PATCH] Let themes declare that they are dark MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The theme extension point gains an "isDarkTheme" attribute, so a theme states its appearance instead of the platform guessing it from the theme id. Themes without the attribute keep the id based classification. ThemeEngine uses ITheme.isDark() for the SWT appearance preference and when picking a dark theme to inherit the operating system setting, so a product shipping its own dark theme works too. The appearance preferences recommend a restart only when switching between light and dark. The flag is persisted next to the theme id, which lets IDEApplication style the workspace selection dialog before the theme engine exists. Assisted-by: multiple AI agents and layers of automated tooling 🤖 --- .../org.eclipse.e4.ui.css.swt.theme.exsd | 7 ++ .../e4/ui/css/swt/internal/theme/Theme.java | 15 ++- .../css/swt/internal/theme/ThemeEngine.java | 95 +++++++++++++++---- .../eclipse/e4/ui/css/swt/theme/ITheme.java | 11 +++ .../ide/application/IDEApplication.java | 13 ++- bundles/org.eclipse.ui.themes/plugin.xml | 4 + .../internal/dialogs/ViewsPreferencePage.java | 62 ++++-------- .../themes/DefaultThemePreference.java | 91 ++++++++++++++++++ .../build.properties | 2 + .../css/testTheme.css | 1 + .../plugin.xml | 26 +++++ .../e4/ui/tests/css/swt/ThemeTest.java | 70 ++++++++++++++ 12 files changed, 324 insertions(+), 73 deletions(-) create mode 100644 bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/themes/DefaultThemePreference.java create mode 100644 tests/org.eclipse.e4.ui.tests.css.swt/css/testTheme.css create mode 100644 tests/org.eclipse.e4.ui.tests.css.swt/plugin.xml diff --git a/bundles/org.eclipse.e4.ui.css.swt.theme/schema/org.eclipse.e4.ui.css.swt.theme.exsd b/bundles/org.eclipse.e4.ui.css.swt.theme/schema/org.eclipse.e4.ui.css.swt.theme.exsd index 5df13746187..7248acb95ad 100644 --- a/bundles/org.eclipse.e4.ui.css.swt.theme/schema/org.eclipse.e4.ui.css.swt.theme.exsd +++ b/bundles/org.eclipse.e4.ui.css.swt.theme/schema/org.eclipse.e4.ui.css.swt.theme.exsd @@ -134,6 +134,13 @@ + + + + whether this theme is a dark theme. Defaults to <code>false</code>, unless the theme id contains "dark", which is the legacy heuristic used before this attribute existed. + + + diff --git a/bundles/org.eclipse.e4.ui.css.swt.theme/src/org/eclipse/e4/ui/css/swt/internal/theme/Theme.java b/bundles/org.eclipse.e4.ui.css.swt.theme/src/org/eclipse/e4/ui/css/swt/internal/theme/Theme.java index 8fc5326cf3d..a48e0b3e311 100644 --- a/bundles/org.eclipse.e4.ui.css.swt.theme/src/org/eclipse/e4/ui/css/swt/internal/theme/Theme.java +++ b/bundles/org.eclipse.e4.ui.css.swt.theme/src/org/eclipse/e4/ui/css/swt/internal/theme/Theme.java @@ -18,11 +18,17 @@ public class Theme implements ITheme { private final String id; private final String label; + private final boolean dark; private String osVersion; public Theme(String id, String label) { + this(id, label, id != null && id.contains("dark")); //$NON-NLS-1$ + } + + public Theme(String id, String label, boolean dark) { this.id = id; this.label = label; + this.dark = dark; } @Override @@ -35,6 +41,11 @@ public String getLabel() { return label; } + @Override + public boolean isDark() { + return dark; + } + public void setOsVersion(String version) { this.osVersion = version; } @@ -45,8 +56,8 @@ public String getOsVersion() { @Override public String toString() { - return "Theme [id=" + id + ", label='" + label + "', osVersion=" - + osVersion + "]"; + return "Theme [id=" + id + ", label='" + label + "', dark=" + dark + + ", osVersion=" + osVersion + "]"; } diff --git a/bundles/org.eclipse.e4.ui.css.swt.theme/src/org/eclipse/e4/ui/css/swt/internal/theme/ThemeEngine.java b/bundles/org.eclipse.e4.ui.css.swt.theme/src/org/eclipse/e4/ui/css/swt/internal/theme/ThemeEngine.java index 27f68c94fb7..57b14178821 100644 --- a/bundles/org.eclipse.e4.ui.css.swt.theme/src/org/eclipse/e4/ui/css/swt/internal/theme/ThemeEngine.java +++ b/bundles/org.eclipse.e4.ui.css.swt.theme/src/org/eclipse/e4/ui/css/swt/internal/theme/ThemeEngine.java @@ -30,6 +30,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import org.eclipse.core.runtime.Assert; import org.eclipse.core.runtime.FileLocator; @@ -89,6 +90,13 @@ public class ThemeEngine implements IThemeEngine { private static final String THEMEID_KEY = "themeid"; + /** + * Prefix of the key recording a theme's appearance. The theme id is part of the + * key, so a record left behind by another installation sharing the preferences + * cannot be mistaken for the appearance of the theme in {@link #THEMEID_KEY}. + */ + private static final String THEME_IS_DARK_KEY_PREFIX = "themeIsDark."; + public static final String THEME_PLUGIN_ID = "org.eclipse.e4.ui.css.swt.theme"; public static final String E4_DARK_THEME_ID = "org.eclipse.e4.ui.css.theme.e4_dark"; @@ -157,13 +165,14 @@ public ThemeEngine(Display display) { final String themeBaseId = id + version; String themeId = themeBaseId; String label = ce.getAttribute("label"); + boolean dark = isDarkTheme(ce); String originalCSSFile; String basestylesheeturi = originalCSSFile = ce.getAttribute("basestylesheeturi"); if (!basestylesheeturi.startsWith("platform:/plugin/")) { basestylesheeturi = "platform:/plugin/" + ce.getContributor().getName() + "/" + basestylesheeturi; } - registerTheme(themeId, label, basestylesheeturi, version); + registerTheme(themeId, label, basestylesheeturi, version, dark); //check for modified files if (modifiedFiles != null) { @@ -237,6 +246,20 @@ public ThemeEngine(Display display) { } + /** + * Themes declare their appearance with the {@code isDarkTheme} attribute. Themes + * contributed before that attribute existed are classified by their id, which is + * the convention the platform relied on so far. + */ + private static boolean isDarkTheme(IConfigurationElement themeElement) { + String dark = themeElement.getAttribute("isDarkTheme"); + if (dark != null) { + return Boolean.parseBoolean(dark); + } + String id = themeElement.getAttribute("id"); + return id != null && id.contains("dark"); //$NON-NLS-1$ + } + private boolean isOsVersionMatch(String osVersionList) { boolean found = false; String osVersion = System.getProperty("os.version"); @@ -264,13 +287,15 @@ public synchronized ITheme registerTheme(String id, String label, String basesty public synchronized ITheme registerTheme(String id, String label, String basestylesheetURI, String osVersion) throws IllegalArgumentException { - for (Theme t : themes) { - if (t.getId().equals(id)) { - throw new IllegalArgumentException("A theme with the id '" + id - + "' is already registered"); - } + return registerTheme(id, label, basestylesheetURI, osVersion, id.contains("dark")); //$NON-NLS-1$ + } + + public synchronized ITheme registerTheme(String id, String label, String basestylesheetURI, String osVersion, + boolean dark) throws IllegalArgumentException { + if (themes.stream().anyMatch(t -> t.getId().equals(id))) { + throw new IllegalArgumentException("A theme with the id '" + id + "' is already registered"); } - Theme theme = new Theme(id, label); + Theme theme = new Theme(id, label, dark); if (osVersion != "") { theme.setOsVersion(osVersion); } @@ -512,6 +537,7 @@ public void setTheme(ITheme theme, boolean restore, boolean force) { EclipsePreferencesHelper.setCurrentThemeId(theme.getId()); pref.put(THEMEID_KEY, theme.getId()); + pref.putBoolean(darkKey(theme.getId()), theme.isDark()); try { pref.flush(); } catch (BackingStoreException e) { @@ -520,8 +546,7 @@ public void setTheme(ITheme theme, boolean restore, boolean force) { } publishEffectiveThemeId(); - boolean isDark = theme.getId().contains("dark"); //$NON-NLS-1$ - display.setDarkThemePreferred(isDark); + display.setDarkThemePreferred(theme.isDark()); sendThemeChangeEvent(restore); @@ -599,7 +624,9 @@ private String getPreferenceThemeId() { */ private void publishEffectiveThemeId() { if (currentTheme != null) { - DefaultScope.INSTANCE.getNode(THEME_PLUGIN_ID).put(THEMEID_KEY, currentTheme.getId()); + IEclipsePreferences defaults = DefaultScope.INSTANCE.getNode(THEME_PLUGIN_ID); + defaults.put(THEMEID_KEY, currentTheme.getId()); + defaults.putBoolean(darkKey(currentTheme.getId()), currentTheme.isDark()); effectiveThemeIdPublished = true; } } @@ -625,24 +652,23 @@ public void restore(String alternateTheme) { // use theme from preferences if it exists if (prefThemeId != null) { - for (ITheme t : getThemes()) { - if (prefThemeId.equals(t.getId())) { - setTheme(t, false); - return; - } + Optional prefTheme = getThemes().stream().filter(t -> prefThemeId.equals(t.getId())).findFirst(); + if (prefTheme.isPresent()) { + setTheme(prefTheme.get(), false); + return; } } - boolean hasDarkTheme = getThemes().stream().anyMatch(t -> t.getId().startsWith(E4_DARK_THEME_ID)); + Optional darkTheme = findDarkTheme(); boolean overrideWithDarkTheme = false; - if (hasDarkTheme) { + if (darkTheme.isPresent()) { if (prefThemeId != null) { /* * The user had previously selected a theme which is not available anymore. In * this case want to fall back to respect whether that previous choice was dark * or not. https://github.com/eclipse-platform/eclipse.platform.ui/issues/2776 */ - overrideWithDarkTheme = prefThemeId.contains("dark"); + overrideWithDarkTheme = isPreferenceThemeDark(prefThemeId); } else { /* * No previous theme selection in preferences. In this case check if the system @@ -654,12 +680,43 @@ public void restore(String alternateTheme) { } } - String themeToRestore = overrideWithDarkTheme ? E4_DARK_THEME_ID : alternateTheme; + String themeToRestore = overrideWithDarkTheme ? darkTheme.get().getId() : alternateTheme; if (themeToRestore != null) { setTheme(themeToRestore, false); } } + private static String darkKey(String themeId) { + return THEME_IS_DARK_KEY_PREFIX + themeId; + } + + /** + * Whether the theme recorded in the preferences is a dark one. Preferences + * written before themes declared their appearance only carry the theme id, so + * fall back to the id based classification. + */ + private boolean isPreferenceThemeDark(String prefThemeId) { + boolean darkById = prefThemeId.contains("dark"); //$NON-NLS-1$ + IPreferencesService prefService = Platform.getPreferencesService(); + if (!effectiveThemeIdPublished) { + return prefService.getBoolean(THEME_PLUGIN_ID, darkKey(prefThemeId), darkById, null); + } + String dark = prefService.get(darkKey(prefThemeId), null, new Preferences[] { + InstanceScope.INSTANCE.getNode(THEME_PLUGIN_ID), ConfigurationScope.INSTANCE.getNode(THEME_PLUGIN_ID), + UserScope.INSTANCE.getNode(THEME_PLUGIN_ID) }); + return dark != null ? Boolean.parseBoolean(dark) : darkById; + } + + /** + * The dark theme to fall back to, preferring the one shipped with the platform + * over a dark theme contributed by someone else. + */ + private Optional findDarkTheme() { + List darkThemes = getThemes().stream().filter(ITheme::isDark).toList(); + return darkThemes.stream().filter(t -> E4_DARK_THEME_ID.equals(t.getId())).findFirst() + .or(() -> darkThemes.stream().findFirst()); + } + @Override public ITheme getActiveTheme() { return currentTheme; diff --git a/bundles/org.eclipse.e4.ui.css.swt.theme/src/org/eclipse/e4/ui/css/swt/theme/ITheme.java b/bundles/org.eclipse.e4.ui.css.swt.theme/src/org/eclipse/e4/ui/css/swt/theme/ITheme.java index 2270a0c9ce3..c6718ab15b6 100644 --- a/bundles/org.eclipse.e4.ui.css.swt.theme/src/org/eclipse/e4/ui/css/swt/theme/ITheme.java +++ b/bundles/org.eclipse.e4.ui.css.swt.theme/src/org/eclipse/e4/ui/css/swt/theme/ITheme.java @@ -26,4 +26,15 @@ public interface ITheme { * @return the label */ String getLabel(); + + /** + * Whether this theme is meant to be used with a dark appearance, as declared by + * the isDarkTheme attribute of the theme extension. Implementations + * that do not override this keep the id based classification the platform used + * before the attribute existed. + */ + default boolean isDark() { + String id = getId(); + return id != null && id.contains("dark"); //$NON-NLS-1$ + } } diff --git a/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEApplication.java b/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEApplication.java index 6ecf3ded110..ae135d0331f 100644 --- a/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEApplication.java +++ b/bundles/org.eclipse.ui.ide.application/src/org/eclipse/ui/internal/ide/application/IDEApplication.java @@ -916,13 +916,12 @@ protected static Version toMajorMinorVersion(Version version) { protected void initializeDefaultTheme(Display display) { IEclipsePreferences themeNode = UserScope.INSTANCE.getNode("org.eclipse.e4.ui.css.swt.theme"); //$NON-NLS-1$ String productOrAppId = getProductOrApplicationId(); - String defaultThemeId; - if (productOrAppId != null) { - defaultThemeId = themeNode.node(productOrAppId).get("themeid", null); //$NON-NLS-1$ - } else { - defaultThemeId = themeNode.get("themeid", null); //$NON-NLS-1$ - } - isDark = defaultThemeId != null && defaultThemeId.contains("dark"); //$NON-NLS-1$ + IEclipsePreferences scopedNode = productOrAppId != null ? (IEclipsePreferences) themeNode.node(productOrAppId) + : themeNode; + String defaultThemeId = scopedNode.get("themeid", null); //$NON-NLS-1$ + // preferences written before themes declared their appearance only carry the id + isDark = defaultThemeId != null + && scopedNode.getBoolean("themeIsDark." + defaultThemeId, defaultThemeId.contains("dark")); //$NON-NLS-1$ //$NON-NLS-2$ if (isDark) { display.setDarkThemePreferred(true); darkThemeShowListener = event -> { diff --git a/bundles/org.eclipse.ui.themes/plugin.xml b/bundles/org.eclipse.ui.themes/plugin.xml index d4dab3dd42c..0430a791a9f 100644 --- a/bundles/org.eclipse.ui.themes/plugin.xml +++ b/bundles/org.eclipse.ui.themes/plugin.xml @@ -5,18 +5,21 @@ point="org.eclipse.e4.ui.css.swt.theme"> diff --git a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/dialogs/ViewsPreferencePage.java b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/dialogs/ViewsPreferencePage.java index 10da19bed2c..cd61235dfd7 100644 --- a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/dialogs/ViewsPreferencePage.java +++ b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/dialogs/ViewsPreferencePage.java @@ -44,7 +44,6 @@ import org.eclipse.core.runtime.preferences.DefaultScope; import org.eclipse.core.runtime.preferences.IEclipsePreferences; import org.eclipse.core.runtime.preferences.InstanceScope; -import org.eclipse.core.runtime.preferences.UserScope; import org.eclipse.e4.core.contexts.IEclipseContext; import org.eclipse.e4.ui.css.swt.theme.ITheme; import org.eclipse.e4.ui.css.swt.theme.IThemeEngine; @@ -89,6 +88,7 @@ import org.eclipse.ui.internal.IWorkbenchHelpContextIds; import org.eclipse.ui.internal.WorkbenchMessages; import org.eclipse.ui.internal.WorkbenchPlugin; +import org.eclipse.ui.internal.themes.DefaultThemePreference; import org.eclipse.ui.internal.themes.IThemeDescriptor; import org.eclipse.ui.internal.util.PrefUtil; import org.eclipse.ui.themes.IThemeManager; @@ -356,10 +356,7 @@ private ITheme getSelectedTheme() { private void openManageDefaultThemeDialog() { String productOrAppId = getProductOrApplicationId(); - IEclipsePreferences baseNode = UserScope.INSTANCE.getNode(E4_THEME_EXTENSION_POINT); - IEclipsePreferences scopedNode = productOrAppId != null ? (IEclipsePreferences) baseNode.node(productOrAppId) - : baseNode; - String currentDefaultId = scopedNode.get("themeid", null); //$NON-NLS-1$ + String currentDefaultId = DefaultThemePreference.getThemeId(); String currentDefaultLabel = null; if (currentDefaultId != null) { @@ -408,21 +405,9 @@ private void openManageDefaultThemeDialog() { int result = dialog.open(); if (result == 0 && selectedTheme != null) { - // Set as default - scopedNode.put("themeid", selectedTheme.getId()); //$NON-NLS-1$ - try { - scopedNode.flush(); - } catch (BackingStoreException e) { - WorkbenchPlugin.log("Failed to set default theme in user scope", e); //$NON-NLS-1$ - } + DefaultThemePreference.set(selectedTheme); } else if (currentDefaultId != null && result == 1) { - // Remove default - scopedNode.remove("themeid"); //$NON-NLS-1$ - try { - scopedNode.flush(); - } catch (BackingStoreException e) { - WorkbenchPlugin.log("Failed to remove default theme from user scope", e); //$NON-NLS-1$ - } + DefaultThemePreference.remove(currentDefaultId); } } @@ -513,11 +498,16 @@ public boolean performOk() { boolean showRestartDialog = false; String restartDialogTitle = null; String restartDialogMessage = null; - boolean themeChanged = false; + ITheme changedTheme = null; if (isThemingPossible()) { ITheme theme = getSelectedTheme(); - themeChanged = theme != null && !theme.equals(currentTheme); + boolean themeChanged = theme != null && !theme.equals(currentTheme); + changedTheme = themeChanged ? theme : null; + // Only a switch between a light and a dark theme leaves parts styled for the + // previous appearance behind, themes of the same appearance restyle in place. + boolean appearanceChanged = themeChanged + && (currentTheme == null || theme.isDark() != currentTheme.isDark()); boolean colorsAndFontsThemeChanged = !PlatformUI.getWorkbench().getThemeManager().getCurrentTheme().getId() .equals(currentColorsAndFontsTheme.getId()); @@ -533,7 +523,7 @@ public boolean performOk() { themeComboDecorator.hide(); colorFontsDecorator.hide(); - if (themeChanged || colorsAndFontsThemeChanged) { + if (appearanceChanged || colorsAndFontsThemeChanged) { showRestartDialog = true; restartDialogTitle = WorkbenchMessages.ThemeChangeWarningTitle; restartDialogMessage = WorkbenchMessages.ThemeChangeWarningText; @@ -551,26 +541,19 @@ public boolean performOk() { } if (showRestartDialog) { - String themeId = null; - if (themeChanged) { - ITheme theme = getSelectedTheme(); - if (theme != null) { - themeId = theme.getId(); - } - } - showRestartDialog(restartDialogTitle, restartDialogMessage, themeId); + showRestartDialog(restartDialogTitle, restartDialogMessage, changedTheme); } return super.performOk(); } - private void showRestartDialog(String title, String warningText, String themeId) { + private void showRestartDialog(String title, String warningText, ITheme theme) { boolean[] useAsDefault = { true }; MessageDialog dialog = new MessageDialog(null, title, null, warningText, MessageDialog.NONE, 2, WorkbenchMessages.Workbench_RestartButton, WorkbenchMessages.Workbench_DontRestartButton) { @Override protected Control createCustomArea(Composite parent) { - if (themeId == null) { + if (theme == null) { return null; } Button checkbox = new Button(parent, SWT.CHECK); @@ -582,19 +565,8 @@ protected Control createCustomArea(Composite parent) { }; int result = dialog.open(); if (result == 0 || result == 1) { // 0: Restart, 1: Don't Restart - if (themeId != null && useAsDefault[0]) { - IEclipsePreferences baseNode = UserScope.INSTANCE - .getNode(E4_THEME_EXTENSION_POINT); - String productOrAppId = getProductOrApplicationId(); - IEclipsePreferences scopedNode = productOrAppId != null - ? (IEclipsePreferences) baseNode.node(productOrAppId) - : baseNode; - scopedNode.put("themeid", themeId); //$NON-NLS-1$ - try { - scopedNode.flush(); - } catch (BackingStoreException e) { - WorkbenchPlugin.log("Failed to set default theme in user scope", e); //$NON-NLS-1$ - } + if (theme != null && useAsDefault[0]) { + DefaultThemePreference.set(theme); } } if (result == 0) { diff --git a/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/themes/DefaultThemePreference.java b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/themes/DefaultThemePreference.java new file mode 100644 index 00000000000..e449df9768a --- /dev/null +++ b/bundles/org.eclipse.ui.workbench/eclipseui/org/eclipse/ui/internal/themes/DefaultThemePreference.java @@ -0,0 +1,91 @@ +/******************************************************************************* + * Copyright (c) 2026 vogella GmbH and others. + * + * 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: + * Lars Vogel - initial API and implementation + *******************************************************************************/ +package org.eclipse.ui.internal.themes; + +import org.eclipse.core.runtime.IProduct; +import org.eclipse.core.runtime.Platform; +import org.eclipse.core.runtime.preferences.IEclipsePreferences; +import org.eclipse.core.runtime.preferences.UserScope; +import org.eclipse.e4.ui.css.swt.theme.ITheme; +import org.eclipse.ui.internal.WorkbenchPlugin; +import org.osgi.service.prefs.BackingStoreException; + +/** + * The theme used for new workspaces and for workspaces without an explicit + * theme. It is recorded in the user scope, so every installation of this + * product sees it, together with the theme's appearance. Readers that run + * before the workbench, like the workspace selection dialog, rely on that + * appearance. + */ +public final class DefaultThemePreference { + + private static final String THEME_PLUGIN_ID = "org.eclipse.e4.ui.css.swt.theme"; //$NON-NLS-1$ + + private static final String THEMEID_KEY = "themeid"; //$NON-NLS-1$ + + /** + * Prefix of the key holding a theme's appearance. The theme id is part of the + * key, so a record left behind by another installation cannot be mistaken for + * the appearance of the theme in {@link #THEMEID_KEY}. + */ + private static final String THEME_IS_DARK_KEY_PREFIX = "themeIsDark."; //$NON-NLS-1$ + + private DefaultThemePreference() { + } + + /** @return the id of the default theme, or {@code null} if none is set */ + public static String getThemeId() { + return getNode().get(THEMEID_KEY, null); + } + + public static void set(ITheme theme) { + IEclipsePreferences node = getNode(); + node.put(THEMEID_KEY, theme.getId()); + node.putBoolean(THEME_IS_DARK_KEY_PREFIX + theme.getId(), theme.isDark()); + flush(node, "Failed to set default theme in user scope"); //$NON-NLS-1$ + } + + public static void remove(String themeId) { + IEclipsePreferences node = getNode(); + node.remove(THEMEID_KEY); + node.remove(THEME_IS_DARK_KEY_PREFIX + themeId); + flush(node, "Failed to remove default theme from user scope"); //$NON-NLS-1$ + } + + private static IEclipsePreferences getNode() { + IEclipsePreferences baseNode = UserScope.INSTANCE.getNode(THEME_PLUGIN_ID); + String productOrAppId = getProductOrApplicationId(); + return productOrAppId != null ? (IEclipsePreferences) baseNode.node(productOrAppId) : baseNode; + } + + /** + * Returns the product ID if a product is configured, otherwise the application + * ID from the system property, or {@code null} if neither is available. + */ + private static String getProductOrApplicationId() { + IProduct product = Platform.getProduct(); + if (product != null) { + return product.getId(); + } + return System.getProperty("eclipse.application"); //$NON-NLS-1$ + } + + private static void flush(IEclipsePreferences node, String errorMessage) { + try { + node.flush(); + } catch (BackingStoreException e) { + WorkbenchPlugin.log(errorMessage, e); + } + } +} diff --git a/tests/org.eclipse.e4.ui.tests.css.swt/build.properties b/tests/org.eclipse.e4.ui.tests.css.swt/build.properties index b7ae18f3a8f..718fb2d6624 100644 --- a/tests/org.eclipse.e4.ui.tests.css.swt/build.properties +++ b/tests/org.eclipse.e4.ui.tests.css.swt/build.properties @@ -16,6 +16,8 @@ source.. = src/ output.. = bin/ bin.includes = META-INF/,\ .,\ + plugin.xml,\ + css/,\ test.xml,\ about.html,\ OSGI-INF/ diff --git a/tests/org.eclipse.e4.ui.tests.css.swt/css/testTheme.css b/tests/org.eclipse.e4.ui.tests.css.swt/css/testTheme.css new file mode 100644 index 00000000000..491a1f88b6c --- /dev/null +++ b/tests/org.eclipse.e4.ui.tests.css.swt/css/testTheme.css @@ -0,0 +1 @@ +/* Empty stylesheet for the themes contributed by this test bundle. */ diff --git a/tests/org.eclipse.e4.ui.tests.css.swt/plugin.xml b/tests/org.eclipse.e4.ui.tests.css.swt/plugin.xml new file mode 100644 index 00000000000..14f4d68804e --- /dev/null +++ b/tests/org.eclipse.e4.ui.tests.css.swt/plugin.xml @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + diff --git a/tests/org.eclipse.e4.ui.tests.css.swt/src/org/eclipse/e4/ui/tests/css/swt/ThemeTest.java b/tests/org.eclipse.e4.ui.tests.css.swt/src/org/eclipse/e4/ui/tests/css/swt/ThemeTest.java index 0a11cb6f44d..3c95dc77cc1 100644 --- a/tests/org.eclipse.e4.ui.tests.css.swt/src/org/eclipse/e4/ui/tests/css/swt/ThemeTest.java +++ b/tests/org.eclipse.e4.ui.tests.css.swt/src/org/eclipse/e4/ui/tests/css/swt/ThemeTest.java @@ -22,6 +22,7 @@ import java.util.Dictionary; import java.util.Hashtable; +import java.util.List; import org.eclipse.core.runtime.Platform; import org.eclipse.core.runtime.preferences.ConfigurationScope; @@ -50,6 +51,11 @@ public class ThemeTest { private static final String THEMEID_KEY = "themeid"; + private static final String PERSISTED_THEME_ID = "persisted.test"; + + // the appearance is recorded under a key carrying the theme id it belongs to + private static final String PERSISTED_THEME_DARK_KEY = "themeIsDark." + PERSISTED_THEME_ID; + @RegisterExtension CssSwtEngine css = new CssSwtEngine(); @@ -133,6 +139,70 @@ void testInheritedThemeIsReadableFromPreferenceService() { } } + @Test + void testDarkAttributeIsReadFromThemeExtension() { + List themes = getThemeEngine(Display.getDefault()).getThemes(); + + assertTrue(findTheme(themes, "org.eclipse.e4.ui.tests.css.swt.theme.declaredDark").isDark()); + assertFalse(findTheme(themes, "org.eclipse.e4.ui.tests.css.swt.theme.declaredLight").isDark()); + // themes contributed before the attribute existed are classified by their id + assertTrue(findTheme(themes, "org.eclipse.e4.ui.tests.css.swt.theme.legacydark").isDark()); + } + + @Test + void testLegacyThemesKeepTheirIdBasedClassification() { + assertTrue(new Theme("com.example.theme.dark", "Legacy dark").isDark()); + assertFalse(new Theme("com.example.theme.light", "Legacy light").isDark()); + // implementations predating isDark() are classified by their id as well + ITheme legacy = new ITheme() { + @Override + public String getId() { + return "com.example.theme.dark"; + } + + @Override + public String getLabel() { + return "Legacy dark"; + } + }; + assertTrue(legacy.isDark()); + } + + @Test + void testDarkFlagIsPersistedWithTheThemeId() { + IThemeEngine themer = getThemeEngine(Display.getDefault()); + IEclipsePreferences node = InstanceScope.INSTANCE.getNode(ThemeEngine.THEME_PLUGIN_ID); + ITheme previousTheme = themer.getActiveTheme(); + String previousId = node.get(THEMEID_KEY, null); + String previousDark = node.get(PERSISTED_THEME_DARK_KEY, null); + + try { + themer.setTheme(new Theme(PERSISTED_THEME_ID, "Persisted", true), true); + + assertEquals(PERSISTED_THEME_ID, node.get(THEMEID_KEY, null)); + assertTrue(node.getBoolean(PERSISTED_THEME_DARK_KEY, false)); + } finally { + if (previousTheme != null) { + themer.setTheme(previousTheme, false); + } + putOrRemove(node, THEMEID_KEY, previousId); + putOrRemove(node, PERSISTED_THEME_DARK_KEY, previousDark); + } + } + + private static void putOrRemove(IEclipsePreferences node, String key, String value) { + if (value != null) { + node.put(key, value); + } else { + node.remove(key); + } + } + + private static ITheme findTheme(List themes, String id) { + return themes.stream().filter(t -> id.equals(t.getId())).findFirst() + .orElseThrow(() -> new AssertionError("Theme not registered: " + id)); + } + private IThemeEngine getThemeEngine(Display display) { IThemeManager manager = context.getService(themeManagerReference); assertNotNull(manager, "Theme manager service not available");