Community
Participate
Working Groups
Created attachment 287232 [details] Screenshot 1. Set macOS system theme to Dark (System Preferences, General, Dark) 2. Launch Eclipse with a fresh workspace (so that there is no "org.eclipse.e4.ui.css.swt.theme.prefs" file) 3. Eclipse is set to its dark theme - this is correct. 4. Quit Eclipse 5. Set macOS system theme to Light (System Preferences, General, Light) 6. Launch Eclipse 7. Eclipse is set to its light theme, *but* the font color on the tabs is too light (see attached screenshot) Seems that some things are written to "org.eclipse.ui.workbench.prefs" that are not set correctly: org.eclipse.ui.workbench.ACTIVE_NOFOCUS_TAB_BG_END=41,41,41 org.eclipse.ui.workbench.ACTIVE_NOFOCUS_TAB_BG_START=43,44,45 org.eclipse.ui.workbench.ACTIVE_NOFOCUS_TAB_TEXT_COLOR=204,204,204 org.eclipse.ui.workbench.ACTIVE_TAB_BG_END=41,41,41 org.eclipse.ui.workbench.ACTIVE_TAB_BG_START=43,44,45 org.eclipse.ui.workbench.ACTIVE_TAB_TEXT_COLOR=221,221,221 org.eclipse.ui.workbench.INACTIVE_TAB_BG_END=49,53,56 org.eclipse.ui.workbench.INACTIVE_TAB_BG_START=59,64,66 org.eclipse.ui.workbench.INACTIVE_TAB_TEXT_COLOR=187,187,187
Exact same symptoms in an RCP app (see https://github.com/gama-platform/gama/issues/3193#issuecomment-937482279). Do you know which component writes these prefs ?
(In reply to Alexis Drogoul from comment #1) > Exact same symptoms in an RCP app (see > https://github.com/gama-platform/gama/issues/3193#issuecomment-937482279). > Do you know which component writes these prefs ? No, but I think it was a relatively recent change. IMHO I think it was not a good idea.
I see this problem as well, whenever I change the Mac system theme from Dark to Light. The change in Bug 564488 causes the colors to be written in the org.eclipse.ui.workbench.prefs file. The problem seems to be that ACTIVE_TAB_TEXT_COLOR=221,221,221 is written in the prefs file for dark theme. But, it's not changed for light theme. All the other colors are changed accordingly for light and dark theme. I'm not sure why ACTIVE_TAB_TEXT_COLOR is not updated for light theme or where to fix this exactly.
> I'm not sure why ACTIVE_TAB_TEXT_COLOR is not updated for light theme or > where to fix this exactly. Dark theme sets the value for this color preference: https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org.eclipse.ui.themes/css/dark/e4-dark_ide_colorextensions.css#n42 and https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org.eclipse.ui.themes/css/dark/e4-dark_ide_colorextensions.css#n166 Light theme doesn't set anything about it https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org.eclipse.ui.themes/css/light/e4-light_ide_colorextensions.css#n23 , so setting the light theme doesn't change the color value, which sticks to earlier value, from dark theme here. A solution would be to make the light theme explicitly set a value for this preference similarly to how dark theme does it.
(In reply to Mickael Istria from comment #4) > > I'm not sure why ACTIVE_TAB_TEXT_COLOR is not updated for light theme or > > where to fix this exactly. > > Dark theme sets the value for this color preference: > https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org. > eclipse.ui.themes/css/dark/e4-dark_ide_colorextensions.css#n42 and > https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org. > eclipse.ui.themes/css/dark/e4-dark_ide_colorextensions.css#n166 > Light theme doesn't set anything about it > https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org. > eclipse.ui.themes/css/light/e4-light_ide_colorextensions.css#n23 , so > setting the light theme doesn't change the color value, which sticks to > earlier value, from dark theme here. > A solution would be to make the light theme explicitly set a value for this > preference similarly to how dark theme does it. I noticed that, too. But what about all the other colors for which the dark theme provides values and the light and system theme don't? Do they show the same behavior of not resetting on a switch from dark to light?
Is the change in Bug #564488 necessary? It seems to be a strange way to handle this.
(In reply to Thomas Wolf from comment #5) > I noticed that, too. But what about all the other colors for which the dark > theme provides values and the light and system theme don't? Do they show the > same behavior of not resetting on a switch from dark to light? Note that light theme != system theme; system theme uses system colors (though SWT symbols), light theme hardcodes some specific colors. But yes, this is in general true that if theme doesn't set a color, then it stays on the same value as it was before the switch, because there is basically no better value to guess. (In reply to Phil Beauvoir from comment #6) > Is the change in Bug #564488 necessary? It seems to be a strange way to > handle this. IIRC, without this change, it was very probable that when switching themes, the color shown in the preference page where not matching the colors in the editor/ColorRegistry. It seems important to keep both preferences and ColorRegistry sync'd for such reason. I don't think it's a bug per se, but it does reveal some other bugs in themes themselves not fully defining their palette.
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/190110
(In reply to Mickael Istria from comment #4) > > I'm not sure why ACTIVE_TAB_TEXT_COLOR is not updated for light theme or > > where to fix this exactly. > > Dark theme sets the value for this color preference: > https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org. > eclipse.ui.themes/css/dark/e4-dark_ide_colorextensions.css#n42 and > https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org. > eclipse.ui.themes/css/dark/e4-dark_ide_colorextensions.css#n166 > Light theme doesn't set anything about it > https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org. > eclipse.ui.themes/css/light/e4-light_ide_colorextensions.css#n23 , so > setting the light theme doesn't change the color value, which sticks to > earlier value, from dark theme here. > A solution would be to make the light theme explicitly set a value for this > preference similarly to how dark theme does it. Thanks Mickael for the pointers. I created a patch with the change and the problem seems to be fixed in Eclipse UI. But, I see that the ACTIVE_TAB_TEXT_COLOR no longer appears in the prefs file when Eclipse is in light theme. It appears only when switched to dark theme.
(In reply to Thomas Wolf from comment #5) > (In reply to Mickael Istria from comment #4) > > > I'm not sure why ACTIVE_TAB_TEXT_COLOR is not updated for light theme or > > > where to fix this exactly. > > > > Dark theme sets the value for this color preference: > > https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org. > > eclipse.ui.themes/css/dark/e4-dark_ide_colorextensions.css#n42 and > > https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org. > > eclipse.ui.themes/css/dark/e4-dark_ide_colorextensions.css#n166 > > Light theme doesn't set anything about it > > https://git.eclipse.org/c/platform/eclipse.platform.ui.git/tree/bundles/org. > > eclipse.ui.themes/css/light/e4-light_ide_colorextensions.css#n23 , so > > setting the light theme doesn't change the color value, which sticks to > > earlier value, from dark theme here. > > A solution would be to make the light theme explicitly set a value for this > > preference similarly to how dark theme does it. > > I noticed that, too. But what about all the other colors for which the dark > theme provides values and the light and system theme don't? Do they show the > same behavior of not resetting on a switch from dark to light? In my testing, I see that only the 7 colors in the comment#0 are written to the prefs file when in dark theme, others are not saved to the file. Not sure why.
Also from a quick test, bug doesn't seem to happen on Windows. Couldn't test on Linux.
(In reply to Lakshmi P Shanmugam from comment #11) > Also from a quick test, bug doesn't seem to happen on Windows. Couldn't test > on Linux. Juts tested on Linux and Windows and the bug occurs there as well.
(In reply to Phil Beauvoir from comment #12) > (In reply to Lakshmi P Shanmugam from comment #11) > > Also from a quick test, bug doesn't seem to happen on Windows. Couldn't test > > on Linux. > > Juts tested on Linux and Windows and the bug occurs there as well. Thanks for checking, Phil!
@All, The patch fixes the problem, but ACTIVE_TAB_TEXT_COLOR is still not set in the prefs file. Not sure why or if it's a problem. Please suggest, does the patch looks good or should I revert the original change instead?
(In reply to Lakshmi P Shanmugam from comment #14) > @All, The patch fixes the problem, but ACTIVE_TAB_TEXT_COLOR is still not > set in the prefs file. Not sure why or if it's a problem. Does it cause any further bug? It could be that the ACTIVE_TAB_TEXT_COLOR is back to the default preference value and is then "unset". > Please suggest, does the patch looks good or should I revert the original > change instead? I think the original change was interesting and useful, and as discussed in comment 7, I believe it was actually fixing a bug by the way with some colors being wrong on preference pages. So if the proposed patch fixes the issue reported here, I don't think we should revert the former one.
(In reply to Mickael Istria from comment #15) > (In reply to Lakshmi P Shanmugam from comment #14) > > @All, The patch fixes the problem, but ACTIVE_TAB_TEXT_COLOR is still not > > set in the prefs file. Not sure why or if it's a problem. > > Does it cause any further bug? It could be that the ACTIVE_TAB_TEXT_COLOR is > back to the default preference value and is then "unset". > I tested on Mac with child eclipse and didn't notice any other issues. > > I think the original change was interesting and useful, and as discussed in > comment 7, I believe it was actually fixing a bug by the way with some > colors being wrong on preference pages. > So if the proposed patch fixes the issue reported here, I don't think we > should revert the former one. Ok, let's try this fix.
Gerrit change https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/190110 was merged to [master]. Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=20935e3cf390a6c6fd422e957e39eca965fb62d6
Fixed in I20220210-1800.
Verified in I20220223-1800
