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 821 files  ±0   1 821 suites  ±0   1h 43m 53s ⏱️ - 8m 48s
 7 725 tests ±0   7 493 ✅  -  2  228 💤 ±0  0 ❌  - 2   4 🔥 + 4 
24 336 runs  ±0  23 577 ✅  - 10  747 💤 ±0  0 ❌  - 2  12 🔥 +12 

For more details on these errors, see this check.

Results for commit 66a430a. ± Comparison against base commit 3fd04b3.

♻️ 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.css.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 3ff3dda186d618639d8a1b59088513ab349ff0cb Mon Sep 17 00:00:00 2001
From: Eclipse Platform Bot <[email protected]>
Date: Wed, 6 Nov 2024 10:51:14 +0000
Subject: [PATCH] Version bump(s) for 4.34 stream


diff --git a/bundles/org.eclipse.e4.ui.css.swt/META-INF/MANIFEST.MF b/bundles/org.eclipse.e4.ui.css.swt/META-INF/MANIFEST.MF
index a10944e052..b6e6c4e55d 100644
--- a/bundles/org.eclipse.e4.ui.css.swt/META-INF/MANIFEST.MF
+++ b/bundles/org.eclipse.e4.ui.css.swt/META-INF/MANIFEST.MF
@@ -1,7 +1,7 @@
 Manifest-Version: 1.0
 Bundle-ManifestVersion: 2
 Bundle-SymbolicName: org.eclipse.e4.ui.css.swt;singleton:=true
-Bundle-Version: 0.15.400.qualifier
+Bundle-Version: 0.15.500.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