Bug 564488 - Update prefs when ColorRegistry updates
Adds a listener that ensures the internal preferences for the color
registry are updated when the color registry is updated via .put(). This
requires a few other code tweaks to ensure that there is no possibility
of infinite loops, since there is also a listener to update the registry
when the prefs are updated. Additionally,
WorkbenchThemeManager.updateThemes has to ensure that all colors in the
registry get properly refreshed, instead of only the ones with
ColorOverride definitions.
Change-Id: Ie6ca55af966aa74e17f536eed0d53dc810ebee6d
Signed-off-by: John Taylor <johnpaultaylorii@gmail.com>
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/Theme.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/Theme.java
index 3bb537f..2d70278 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/Theme.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/Theme.java
@@ -86,11 +86,45 @@
}
getColorRegistry().addListener(getCascadeListener());
+ getColorRegistry().addListener(this::registryColorChangeEvent);
getFontRegistry().addListener(getCascadeListener());
PrefUtil.getInternalPreferenceStore().addPropertyChangeListener(getPropertyListener());
}
/**
+ * When a color in the registry is updated, update the backing preferences
+ * appropriately.
+ *
+ * @param event the color change event
+ */
+ private void registryColorChangeEvent(PropertyChangeEvent event) {
+ if (event.getNewValue() instanceof RGB) {
+ String key = ThemeElementHelper.createPreferenceKey(this, event.getProperty());
+ IPreferenceStore store = PrefUtil.getInternalPreferenceStore();
+ if (store.contains(key)) {
+ RGB newColor = (RGB) event.getNewValue();
+ if (store.isDefault(key)) {
+ RGB oldColor = PreferenceConverter.getDefaultColor(store, key);
+ if (oldColor == PreferenceConverter.COLOR_DEFAULT_DEFAULT) {
+ // If the preference is set to default, but there is no actual default value,
+ // then the preference state is inconsistent. Do nothing.
+ } else if (!newColor.equals(oldColor))
+ PreferenceConverter.setValue(store, key, newColor);
+ } else {
+ RGB oldColor = PreferenceConverter.getColor(store, key);
+ if (!newColor.equals(oldColor)) {
+ oldColor = PreferenceConverter.getDefaultColor(store, key);
+ if (oldColor != PreferenceConverter.COLOR_DEFAULT_DEFAULT && newColor.equals(oldColor))
+ store.setToDefault(key);
+ else
+ PreferenceConverter.setValue(store, key, newColor);
+ }
+ }
+ }
+ }
+ }
+
+ /**
* Listener that is responsible for responding to preference changes.
*
* @return the property change listener
@@ -118,15 +152,14 @@
getFontRegistry().put(key, data);
processDefaultsTo(key, data);
- return;
} else if (getColorRegistry().hasValueFor(key)) {
RGB rgb = event.getNewValue() instanceof String
? StringConverter.asRGB((String) event.getNewValue())
: (RGB) event.getNewValue();
-
- getColorRegistry().put(key, rgb);
- processDefaultsTo(key, rgb);
- return;
+ if (!Objects.equals(getColorRegistry().getRGB(key), rgb)) {
+ getColorRegistry().put(key, rgb);
+ processDefaultsTo(key, rgb);
+ }
}
}
} catch (DataFormatException e) {
@@ -167,8 +200,10 @@
String defaultsTo = colorDefinition.getDefaultsTo();
if (defaultsTo != null && defaultsTo.equals(key)) {
IPreferenceStore store = WorkbenchPlugin.getDefault().getPreferenceStore();
- if (store.isDefault(
- ThemeElementHelper.createPreferenceKey(Theme.this, colorDefinition.getId()))) {
+ String prefkey = ThemeElementHelper.createPreferenceKey(Theme.this,
+ colorDefinition.getId());
+ if (store.isDefault(prefkey)) {
+ PreferenceConverter.setDefault(store, prefkey, rgb);
getColorRegistry().put(colorDefinition.getId(), rgb);
processDefaultsTo(colorDefinition.getId(), rgb);
}
@@ -213,6 +248,7 @@
public void dispose() {
if (themeColorRegistry != null) {
themeColorRegistry.removeListener(themeListener);
+ themeColorRegistry.removeListener(this::registryColorChangeEvent);
themeColorRegistry.dispose();
}
if (themeFontRegistry != null) {
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/ThemeElementHelper.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/ThemeElementHelper.java
index 9e78d17..c1391c5 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/ThemeElementHelper.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/ThemeElementHelper.java
@@ -16,6 +16,7 @@
import java.util.Arrays;
import java.util.SortedSet;
import java.util.TreeSet;
+import org.eclipse.core.runtime.Assert;
import org.eclipse.jface.preference.IPreferenceStore;
import org.eclipse.jface.preference.PreferenceConverter;
import org.eclipse.jface.resource.ColorRegistry;
@@ -274,15 +275,12 @@
private static void installColor(ColorDefinition definition, ITheme theme, IPreferenceStore store,
boolean setInRegistry) {
-
- // TODO: store shouldn't be null, should assert instead of checking null all
- // over
+ Assert.isNotNull(store);
ColorRegistry registry = theme.getColorRegistry();
String id = definition.getId();
String key = createPreferenceKey(theme, id);
- RGB prefColor = store != null ? PreferenceConverter.getColor(store, key) : null;
RGB defaultColor;
if (definition.getValue() != null) {
defaultColor = definition.getValue();
@@ -290,17 +288,11 @@
String defaultsToKey = createPreferenceKey(theme, definition.getDefaultsTo());
defaultColor = PreferenceConverter.getDefaultColor(store, defaultsToKey);
} else {
- defaultColor = null;
- }
-
- if (defaultColor == null) {
- // default is null, likely because we have a bad definition - the
- // defaultsTo color doesn't exist. We still need a sensible default,
- // however.
defaultColor = PreferenceConverter.COLOR_DEFAULT_DEFAULT;
}
- if (prefColor == null || prefColor == PreferenceConverter.COLOR_DEFAULT_DEFAULT) {
+ RGB prefColor = PreferenceConverter.getColor(store, key);
+ if (prefColor == PreferenceConverter.COLOR_DEFAULT_DEFAULT || store.isDefault(key)) {
if (definition.getValue() != null) {
prefColor = definition.getValue();
} else if (definition.getDefaultsTo() != null) {
@@ -312,13 +304,13 @@
prefColor = defaultColor;
}
- if (setInRegistry) {
- registry.put(id, prefColor);
- }
-
if (store != null) {
PreferenceConverter.setDefault(store, key, defaultColor);
}
+
+ if (setInRegistry) {
+ registry.put(id, prefColor);
+ }
}
/**
diff --git a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/WorkbenchThemeManager.java b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/WorkbenchThemeManager.java
index 3168252..2e03bc9 100644
--- a/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/WorkbenchThemeManager.java
+++ b/bundles/org.eclipse.ui.workbench/Eclipse UI/org/eclipse/ui/internal/themes/WorkbenchThemeManager.java
@@ -189,11 +189,11 @@
ITheme theme = themes.get(themeDescriptor);
// If theme is in our themes table then its already been populated
if (theme != null) {
- ColorDefinition[] colorDefinitions = themeDescriptor.getColors();
-
- if (colorDefinitions.length > 0) {
- ThemeElementHelper.populateRegistry(theme, colorDefinitions, PrefUtil.getInternalPreferenceStore());
- }
+ // By the time updateThemes is called, all colors have overrides in the
+ // CascadingColorRegistry (due to Workbench.initializeApplicationColors), so
+ // we need to repopulate using getColorsFor() to update everything.
+ ThemeElementHelper.populateRegistry(theme, getThemeRegistry().getColorsFor(theme.getId()),
+ PrefUtil.getInternalPreferenceStore());
}
}
}
diff --git a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/themes/ThemeAPITest.java b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/themes/ThemeAPITest.java
index a4564f5..5702830 100644
--- a/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/themes/ThemeAPITest.java
+++ b/tests/org.eclipse.ui.tests/Eclipse UI Tests/org/eclipse/ui/tests/themes/ThemeAPITest.java
@@ -296,6 +296,70 @@
}
@Test
+ public void testColorRegistryListener_def_swtcolor() {
+ IPreferenceStore store = PrefUtil.getInternalPreferenceStore();
+ ITheme defaultTheme = getDefaultTheme();
+
+ testColorRegistryPut(defaultTheme, store, SWTCOLOR);
+ }
+
+ @Test
+ public void testColorRegistryListener_def_rgbcolor() {
+ IPreferenceStore store = PrefUtil.getInternalPreferenceStore();
+ ITheme defaultTheme = getDefaultTheme();
+
+ testColorRegistryPut(defaultTheme, store, RGBCOLOR);
+ }
+
+ @Test
+ public void testColorRegistryListener_def_defaultedcolor() {
+ IPreferenceStore store = PrefUtil.getInternalPreferenceStore();
+ ITheme defaultTheme = getDefaultTheme();
+
+ testColorRegistryPut(defaultTheme, store, DEFAULTEDCOLOR);
+ }
+
+ @Test
+ public void testColorRegistryListener_def_nooverridecolor() {
+ IPreferenceStore store = PrefUtil.getInternalPreferenceStore();
+ ITheme defaultTheme = getDefaultTheme();
+
+ testColorRegistryPut(defaultTheme, store, NOOVERRIDECOLOR);
+ }
+
+ @Test
+ public void testColorRegistryListener_th1_swtcolor() {
+ IPreferenceStore store = PrefUtil.getInternalPreferenceStore();
+ ITheme theme1 = getTheme1();
+
+ testColorRegistryPut(theme1, store, SWTCOLOR);
+ }
+
+ @Test
+ public void testColorRegistryListener_th1_rgbcolor() {
+ IPreferenceStore store = PrefUtil.getInternalPreferenceStore();
+ ITheme theme1 = getTheme1();
+
+ testColorRegistryPut(theme1, store, RGBCOLOR);
+ }
+
+ @Test
+ public void testColorRegistryListener_th1_defaultedcolor() {
+ IPreferenceStore store = PrefUtil.getInternalPreferenceStore();
+ ITheme theme1 = getTheme1();
+
+ testColorRegistryPut(theme1, store, DEFAULTEDCOLOR);
+ }
+
+ @Test
+ public void testColorRegistryListener_th1_nooverridecolor() {
+ IPreferenceStore store = PrefUtil.getInternalPreferenceStore();
+ ITheme theme1 = getTheme1();
+
+ testColorRegistryPut(theme1, store, NOOVERRIDECOLOR);
+ }
+
+ @Test
public void testDataKeySet_data1() {
ITheme defaultTheme = getDefaultTheme();
Set<String> themeKeys = defaultTheme.keySet();
@@ -538,6 +602,19 @@
}
+ private void testColorRegistryPut(ITheme theme, IPreferenceStore store, String color) {
+ String key = ThemeElementHelper.createPreferenceKey(theme, color);
+ RGB oldRGB = PreferenceConverter.getColor(store, key);
+ RGB newRGB = new RGB(57, 12, 86);
+
+ theme.getColorRegistry().put(color, newRGB);
+ assertEquals(newRGB, store.isDefault(key) ? PreferenceConverter.getDefaultColor(store, key)
+ : PreferenceConverter.getColor(store, key));
+ theme.getColorRegistry().put(color, oldRGB);
+ assertEquals(oldRGB, store.isDefault(key) ? PreferenceConverter.getDefaultColor(store, key)
+ : PreferenceConverter.getColor(store, key));
+ }
+
private void testOverrideColorPreference(ITheme theme,
IPreferenceStore store, String color) {
RGB oldRGB = theme.getColorRegistry().getRGB(color);
