Bug 564488

Summary:	Changes to ColorRegistry are not reflected in internal preferences
Product:	[Eclipse Project] Platform	Reporter:	John Taylor <johnpaultaylorii>
Component:	UI	Assignee:	Platform-UI-Inbox <Platform-UI-Inbox>
Status:	RESOLVED FIXED	QA Contact:
Severity:	minor
Priority:	P3	CC:	aobuchow, lshanmug, mistria
Version:	4.17
Target Milestone:	---
Hardware:	All
OS:	All
See Also:	https://git.eclipse.org/r/165238 https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=433a7285783c47612cc62a7b385253449247caa2 https://bugs.eclipse.org/bugs/show_bug.cgi?id=576331 https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/190412
Whiteboard:
Bug Depends on:
Bug Blocks:	531033

Description John Taylor

2020-06-19 14:31:03 EDT

When a call is made to ColorRegistry#put(String, RGB), the preferences for that item in the WorkbenchPlugin's store are not updated. Ordinarily, this doesn't really present a problem. However, in order to implement Bug 531033, the prefs from EditorsPlugin need to update the registry, which needs to be backed by the WorkbenchPlugin's prefs. If these are not in sync then, for example, changing the Windows Theme (invoking WorkbenchThemeManager#updateThemes()) causes the previous values to be used instead of the current values.

Patch proposal incoming shortly.

Comment 1 John Taylor

2020-06-19 14:35:11 EDT

Question, for those who know more about this than me. The only reason for adding this synchronization is because EditorsPlugin needs to update colors in the registry, which is owned by WorkbenchPlugin. Since synchronization from WorkbenchPlugin's prefs to ColorRegistry already exists, if I could just update the WorkbenchPlugin's prefs, Bug 531033 could be resolved without this. So, my question is - is there any sanctioned way to update prefs for another plugin, or is it preferred to only use public API (in this case, ColorRegistry.put)?

Comment 2 Eclipse Genie

2020-06-19 14:50:22 EDT

New Gerrit change created: https://git.eclipse.org/r/165238

Comment 3 Eclipse Genie

2020-06-22 03:52:09 EDT

Gerrit change https://git.eclipse.org/r/165238 was merged to [master].
Commit: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=433a7285783c47612cc62a7b385253449247caa2

Comment 4 Mickael Istria

2020-06-22 03:52:22 EDT

Thanks!

Comment 5 Lakshmi P Shanmugam

2022-01-26 12:34:54 EST

This change caused Bug 576331 as a side-effect. Any inputs on how to fix it?

Comment 6 Eclipse Genie

2022-02-04 10:58:05 EST

New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/190412

Comment 7 Mickael Istria

2022-02-04 11:14:19 EST

@John: what do you think? IIRC your patch was fixing some existing bugs with theme and pref page being out of sync, wasn't it?
If not, then we can probably revert without causing issues.
If yes, then I think it's really a matter of choosing between 2 bugs, and I would recommend choosing the one that seems to have the more correct implementation.