Reset in colors preference page now respects theme-specific defaults

[tobiasmelcher]

The "Reset" button in the colors and fonts preference page does not take the default colors overridden in the theme specific css files into account.
Example:
org.eclipse.ui.editors.inlineAnnotationColor is specified with 128,128,128 in plugin.xml

<colorDefinition
            categoryId="org.eclipse.ui.workbenchMisc"
            id="org.eclipse.ui.editors.inlineAnnotationColor"
            isEditable="true"
            label="%TEXT_EDITOR_CODE_MINING_COLOR"
            value="128,128,128">
        <description>
            %TEXT_EDITOR_CODE_MINING_COLOR_DESCRIPTION
         </description>
</colorDefinition>

and for the dark theme e4-dark_preferencestyle.css it is overridden with 155,155,155:

IEclipsePreferences#org-eclipse-ui-workbench:org-eclipse-ui-editors { 
	preferences:
		'org.eclipse.ui.editors.inlineAnnotationColor=155,155,155'
}

I would have expected that click on "Reset" button in the colors preference page sets the color to 155,155,155 in the dark theme which is not the case.

(Reset button is disabled, so default value is visible which is 128,128,128 instead 155,155,155)
This pull requests changes the behavior and sets the default color to 155,155,155 in the dark theme and to 128,128,128 in the light theme.

There already exists a method RGB getColorValue(ColorDefinition definition) in ColorsAndFontsPreferencePage but it was not working. I now created a second method RGB getColorRGB(ColorDefinition definition) which works as I expect; it returns the default color from plugin.xml if no theme is set or the color from the css theme if available.

Is the changed behavior in this pull request ok? Could you please explain the intent of the old method? Should we try to merge the behavior of the two methods?
Any guidance you can provide would be greatly appreciated. Thank you very much!

[BeckerWdf]

This was also reported in https://bugs.eclipse.org/bugs/show_bug.cgi?id=478835

[BeckerWdf]

what happens if i press "Restore Defaults" while in the dark theme and later on switch back to light theme.
Does the colors then switch to the defaults of the light theme or do they stay on the default values from the dark theme as if the user would have set them manually?

[github-actions]

Test Results

 1 214 files  ±0   1 214 suites  ±0   1h 15m 4s ⏱️ + 4m 59s
 7 734 tests ±0   7 501 ✅ ±0  233 💤 ±0  0 ❌ ±0 
16 242 runs  ±0  15 727 ✅ ±0  515 💤 ±0  0 ❌ ±0 

Results for commit 47be4f6. ± Comparison against base commit b019337.

♻️ This comment has been updated with latest results.

[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.e4.ui.workbench.swt/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 6249ce3c5aa42405f609059d62c1b1f14f5d0b72 Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Wed, 27 Nov 2024 14:33:16 +0000
Subject: [PATCH] Version bump(s) for 4.35 stream


diff --git a/bundles/org.eclipse.e4.ui.workbench.swt/META-INF/MANIFEST.MF b/bundles/org.eclipse.e4.ui.workbench.swt/META-INF/MANIFEST.MF
index c15044ab33..ce84f6bee7 100644
--- a/bundles/org.eclipse.e4.ui.workbench.swt/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.e4.ui.workbench.swt/META-INF/MANIFEST.MF
@@ -1,7 +1,7 @@
 Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-SymbolicName: org.eclipse.e4.ui.workbench.swt;singleton:=true
-Bundle-Version: 0.17.600.qualifier
+Bundle-Version: 0.17.700.qualifier
 Bundle-Name: %pluginName
 Bundle-Vendor: %providerName
 Bundle-Localization: plugin
-- 
2.47.0

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

[BeckerWdf]

https://bugs.eclipse.org/bugs/show_bug.cgi?id=478835

Your PR does if the issue in the "Fonts & Colors" preference page but is does not fix the issue reported in https://bugs.eclipse.org/bugs/show_bug.cgi?id=478835 (on JDTs "Syntax Color" preference page).

[BeckerWdf]

I did a manual test round and this looks good. Bugs have been fixed.
Still I find issues but they already exist in today's master. Will create new issues for these as they can be fixed separately.

Testcases

"Colors And Fonts" Preference Page

1) ✅

Change color values in light theme.
Check that "Reset" button gets enabled ✅
Check that correct values is set when pressing "Reset" ✅

2) ✅

In "Light" theme set a user defined color value.
Press "Apply".
Change theme to "Dark".
Press "Apply" but don't restart.
Check color value are correct default from dark theme. ✅

This did not work correctly before this PR

3) ✅

In dark theme change the the color value.
Press "Reset" and check that default value from dark theme is set (and not from the light) theme. ✅

This did not work correctly before this PR

4) ❌

In “Dark” theme set a user defined color value.
Press “Apply”.
Change theme to “Light”.
Press “Apply” but don’t restart.
Check color value are correct default from light theme.

User defined value still there. Re-opening the preference dialog or restarting the IDE does not change this. ❌
Put pressing "Reset" does the trick.
Behaviour did not change with this PR.
See: #2500

JDT's "Syntax Colors" Preference Page

5) ✅

In "Light" theme enable "Classes" color and set a user defined color and set font to bold
Press "Restore Defaults". ✅

6) ❌

In “Light” theme enable “Classes” color and set a user defined color.
Press “Apply”.
Change theme to “Dark”.
Press “Apply” but don’t restart.
Check color value as correct default from dark theme.

We still see the user's value. ❌
Re-opening the Preference Dialog fixes this. So it's "only" a UI refresh issue.
Behaviour did not change with this PR.
See: eclipse-jdt/eclipse.jdt.ui#1770

7) ✅

In “Dark” theme enable “Classes” color and set a user defined color.
Press “Apply”.
Press "Restore Defaults"
Set's the defaults of the Dark theme ✅

8) ❌

In Light theme change to dark theme press "Apply" but don't restart and don't close the preference dialog
In “Dark” theme set “Classes” color to a user defined color.
Press “Restore Defaults”

We still see the user’s value. ❌
Re-opening the Preference Dialog fixes this. So it’s “only” a UI refresh issue.
Behaviour did not change with this PR.
See: eclipse-jdt/eclipse.jdt.ui#1770

[BeckerWdf]

Let's merge this early in 4.35

[BeckerWdf]

There are some test failing:

org.eclipse.e4.ui.css.swt.properties.preference.EclipsePreferencesHandlerTest.testCustomizePreferenceOverriddenByCSS
org.eclipse.e4.ui.css.swt.properties.preference.EclipsePreferencesHandlerTest.testOverridePropertyWithNameAndValueSplit
UiTestSuite org.eclipse.ui.tests.themes.ThemesTestSuite org.eclipse.ui.tests.themes.StylingPreferencesHandlerTest.testHandleEvent
UiTestSuite org.eclipse.ui.tests.themes.ThemesTestSuite org.eclipse.ui.tests.themes.StylingPreferencesHandlerTest.testResetOverriddenPreferences

@tobiasmelcher: Can you pls. check?

[BeckerWdf]

I assume that the failing RcpTestSuite PlatformUITest.testCreateAndRunWorkbench is unrelated.
@tobiasmelcher: Can you pls. check?

[tobiasmelcher]

testCreateAndRunWorkbench

org.eclipse.ui.tests.rcp.PlatformUITest fails on my local windows machine with the master branch and with the changes of this pull request. The same assert errors are thrown in the test run. with both branches. So, the test error is not related to this change.

[BeckerWdf]

failing API checks on Linux are known.