Community
Participate
Working Groups
Build ID: 3.4 M4 Steps To Reproduce: We are adding org.eclipse.ui/CURRENT_THEME_ID=DefaultTheme in our plugin_customization.ini but WorkbenchThemeManager is overriding this value. The methodology of setting our theme in plugin_customization.ini worked for us in 3.2.2. We are in the process of moving up to 3.4 and are running into this problem. In 3.2.2 WorkbenchThemeManager code looked like this: private WorkbenchThemeManager() { defaultThemeColorRegistry = new ColorRegistry(PlatformUI.getWorkbench() .getDisplay()); defaultThemeFontRegistry = new FontRegistry(PlatformUI.getWorkbench() .getDisplay()); //copy the font values from preferences. FontRegistry jfaceFonts = JFaceResources.getFontRegistry(); for (Iterator i = jfaceFonts.getKeySet().iterator(); i.hasNext();) { String key = (String) i.next(); defaultThemeFontRegistry.put(key, jfaceFonts.getFontData(key)); } } In 3.4 the following code was added to it: String themeId = IThemeManager.DEFAULT_THEME; // Check if we are in high contrast mode. If so then set the theme to // the system default if (PlatformUI.getWorkbench().getDisplay() != null) { // Determine the high contrast setting before // any access to preferences final boolean[] highContrast = new boolean[] { false }; PlatformUI.getWorkbench().getDisplay().syncExec(new Runnable() { /* * (non-Javadoc) * * @see java.lang.Runnable#run() */ public void run() { highContrast[0] = Display.getCurrent().getHighContrast(); Display.getCurrent().addListener(SWT.Settings, new Listener() { public void handleEvent(Event event) { updateThemes(); } }); } }); if (highContrast[0]) themeId = SYSTEM_DEFAULT_THEME; } PrefUtil.getAPIPreferenceStore().setDefault( IWorkbenchPreferenceConstants.CURRENT_THEME_ID, themeId); The new code that was added is overriding our theme with the value here which is : org.eclipse.ui.defaultTheme More information:
Tod: did you intentionally want to force the theme to IThemeManager.DEFAULT_THEME in the event that we weren't running in high contrast?
No - I expect this was an issue with kevin's cleanup in M4
Kevin: can you chime in on this please?
As I recall in that method I only added Display.getCurrent().addListener(SWT.Settings, new Listener() { public void handleEvent(Event event) { updateThemes(); } }); but I will have a look at it.
Tod: actually, looking at the history I see that this change came in as part of the fix for bug 183036 which you checked in for version 1.24.
Tod: this bug exists in 3.3. Should this be a 3.3.2 candidate?
3.3.2 is now at RC1 so unless it is critical we shold not.
We are currently on 3.2.2 and are jumping directly to 3.4. We have skipped the intermediate releases. So for us we just need it fixed in 3.4. We don't need this fixed in 3.3.2.
Any news on this?
In particular, this is still targeted as 3.4 M5, which seems unlikely at this point.
D'oh. I will look at this start of M7 (I think I got this as part of the great Tod bug move).
The code doesn't take into account the fact that the default theme could be set via product_customization.ini, which happens before this code gets run. The coding change is simple but its not clear what the right behaviour should be for High Contrast: 1) If we detect HC we can set the default to the system default and thus get the user will get the OS HC colors and fonts. However, this will ignore all values that the product specific theme, including not only fonts and colors (which could be acceptable in the name of accessibility) but also any other <data> that might be set, which is a problem. 2) We can check for an observe an existing theme preference (e.g. via product_customization.ini). But then in HC mode there'll be no color or font changes. This could cause the product to fail accessibility testing.' Requires more thought.
A solution which I think should work is for the RCP application to add all its <data> bindings to the root theme, which all the others inherit from. We then follow strategy (1) below, but a switch to the HC would preserve those <data> values because the HC simply does <fontOverride>'s and <colorOverride>'s to get the system colors, and otherwise inherits all other theme values.
Matt, Hiro, can you comment?
On further reflection and with discussion with Tod, I believe my comment #13 is the right approach, which is that the RCP application should move its <data> bindings up to the default theme so that they're inherited regardless of the theme in use. Q: How did your RCP application work in High Contrast? Looking at our old code, I believe that anybody who overrode the default theme in the ini wouldn't then pick up the right font and colors from the OS when in HC (ie. would fail accessibility). This was one reason for our code changes. This means that when in HC you'd always get the system fonts and colors, but the rest of your theme values would be preserved. I assume this is what you want. The other solution path is that we just always observe the theme in the ini regardless of whether we're in HC or not. But then for your RCP app to work nice in HC, you'd need to do a lot more work for your font and color defs since you'd need to take on the problem of the HC check and special casing of values.
Btw, additional tweaking for you if we go with solution (1): I noticed in your plugin.xml you have some additional color definitions. Since they're against the root theme (good), you should ideally be adding <colorOverride>'s to the system default theme with HC friendly values.
Created attachment 94900 [details] Patch to hold onto the change I've implemented strategy (1) as described. Its attached as patch but I've not released pending resolution of the discussion.
I would like to hear back from the reporting team ASAP if this solution is acceptable since the item is marked major I'd like to commit the code as early in the milestone as possible.
Kevin, I am sorry for the delay. We are investigating if this will work for us.
Kevin, Can we have a pref for the default high contrast theme, like org.eclipse.ui/CURRENT_HIGH_CONTRAST_THEME_ID? I think by default it can use SYSTEM_DEFAULT_THEME for high contrast mode. But our RCP app may have multiple themes both for normal mode and high contrast mode in the future, and if Eclipse provides CURRENT_HIGH_CONTRAST_THEME_ID, it would give us more flexibilities to handle the theme. Thanks, Hiro
Hiro Could you log a seperate request for that feature please? We don't want it to get lost when this bug is fixed.
(In reply to comment #20) Hiro, I had thought of a separate key for specifying and overriding the HC theme, but I figured it was more work than people were generally willing to put in for HC. Thanks for proving me wrong! :) I just want to clarify whether this additional HC key request is required for you to close off on this bug. You used the word "in the future", which leads me to believe that - you'd be ok with the current proposed solution - but come say 3.5 you'd like to have the flexibility of a custom HC theme. Please confirm thanks.
(In reply to comment #21) Todd, created bug 226139
(In reply to comment #22) Kevin, we would like Eclipse to have the pref for 3.4, since it is just add a pref, instead of having the hard-coded themeId SYSTEM_DEFAULT_THEME, so that we can keep our current theme without modifying it, and can have our own HC theme separately.
Hiro, this bug is marked sev "Major". You used the word "like" for #226139 which is marked sev "normal" and isn't marked as blocking this one. That says to me #226139 is nice to have but not absolutely required for 3.4. Can you confirm? I just want to differentiate "like" from "must have". Clearly fixing this present bug is "must have". We're in API freeze and its the end of the release cycle so we need to differentiate by severity. Thanks.
Matt, any comments ?
I'm jumping a little late but I think the ideal solution would be to only override font and colors when a switch to high contrast occurs. Since themes support arbitrary data name/value pairs I don't think it's a good solution to a) force users to contribute to the root or b) wipe out the data on a switch to high contrast. Would it be reasonable to just write some wrappers around the Font and Color registries that the theme manager uses? These wrappers would return null when in high contrast mode.
(In reply to comment #27) > I'm jumping a little late but I think the ideal solution would be to only > override font and colors when a switch to high contrast occurs. >... Unfortunately that's a non trivial coding and design change. We just can't do that in M7, and even for 3.5 it'd require a lot more consideration. Given our current technology, I think the ideal solution is the one Hiro proposed, which is for us to provide a 2nd key for specifying the HC theme. But even in that case, I believe you'd still want to put your <data> tags on the base theme so that they're shared between your Default and your new HC theme. Otherwise you'd need to duplicate them (assuming they have the same values in both). So a workable immediate solution is the one proposed by this bug. >>I don't think it's a good solution to ... force users to contribute to the root I actually believe this is in fact the right solution, because your RCP won't look right without those keys. Consider that our base theme has everything in it to ensure the workbench will run and look ok. By adding new <data> tags your adding to the set of things required for your RCP app to work. Thus they too should go in the base. This ensures for example that if you picked up a theme from elsewhere, or allowed users to define their own, etc., your RCP application would still work. So what I concluded was that the moving of your <data> fields to the base theme is the right solution, and as a bonus you will work ok in HC, which I don't think you did previously. In 3.5 we can add the HC theme key for the ini but even if we added it today you'd still as I said want to move your data up to avoid duplication. I'd of course prefer a solution where your code just kept working as written, but to get the same behaviour as before would be solution (2) in my comment #12. No changes on your part, including you continuing to not work in HC. So, do you want no code changes or do you want to work in HC? I think those are the choices. Naturally I believe not working in HC is a terrible solution for you and others. Clearly all this require a complete rethink for 4.0 though.
Timed out on the conversation so released in 20080414. Please reopen if you have further concerns.
