Bug 553029 - Add preference listener to retheme Eclipse if theme preference changes
Summary: Add preference listener to retheme Eclipse if theme preference changes
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.14   Edit
Hardware: PC Linux
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Platform-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: helpwanted, usability
Depends on:
Blocks:
 
Reported: 2019-11-14 03:01 EST by Lars Vogel CLA
Modified: 2021-11-19 01:02 EST (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Lars Vogel CLA 2019-11-14 03:01:37 EST
Oomphs preference recorder does not apply correctly theme changes. 

To reproduce:

1.) Install and activate preference recorder
2.) In a new workspace switch your theme (either to dark or to light)
3.) Open another workspace which used the opposite theme -> theme gets a weird combination of white and dark
4.) Restart Eclipse for the messy looking workspace -> theme gets correctly applied
Comment 1 Ed Merks CLA 2019-11-14 03:53:41 EST
One might argue that such a restart shouldn't be required in the first place, but I don't realistically expect that platform and the stack built on it to accommodate such an expectation. :-P

I'll bet though that if one does this same process manually, i.e., import preferences from a preference file that the platform also doesn't suggest that a restart is needed... Please contradict me if I lose this bet. :-P

In any case, the preference task implementation has no idea what any of the preferences actually mean, so knowledge about this specific preference requiring a restart in order to work properly is something that would need to be hard coded either in the task itself or via some extended information (an annotation) perhaps on the task; in the latter case, the preference recorder would still need to create the annotation so there would still be hard-coded logic for this specific preference somewhere in the code...

I'll mark this as an enhancement given that this is asking for specific new handling for this one specific preference.
Comment 2 Lars Vogel CLA 2019-11-14 04:05:18 EST
The platform does not require a restart after theme switch (if Oomph is not present). We still warn about it but at least on Linux dynamic theme switching works fine without restart.
Comment 3 Ed Merks CLA 2019-11-14 04:15:01 EST
I know the platform requires a restart for this preference and tells you that that when you use the preference dialog.  The question was whether the platform does anything helpful when the user manually imports preferences?
Comment 4 Lars Vogel CLA 2019-11-14 04:23:57 EST
(In reply to Ed Merks from comment #3)
> I know the platform requires a restart for this preference and tells you
> that that when you use the preference dialog.  

No, it does not required a restart at theme switching. We used to require that and therefore we implemented the warning. The only reason why this warning is left on, is that certain SWT widgets might still have bugs so that they do not react to our restyling request.

To test this: Preference -> General -> Appearance. Switch from Light to Dark -> Should work fine without restart on Mac.

> The question was whether the
> platform does anything helpful when the user manually imports preferences?

I almost never did import preference but I guess preference listeners are triggered during this import? If yes, we could register a preference listener for theme changes, which should also help Oomph. Please confirm, in this case, I move this bug to platform.
Comment 5 Ed Merks CLA 2019-11-14 05:23:21 EST
(In reply to Lars Vogel from comment #4)
> (In reply to Ed Merks from comment #3)
> > I know the platform requires a restart for this preference and tells you
> > that that when you use the preference dialog.  
> 
> No, it does not required a restart at theme switching. We used to require
> that and therefore we implemented the warning. The only reason why this
> warning is left on, is that certain SWT widgets might still have bugs so
> that they do not react to our restyling request.
> 

Okay, well the warning sure made me believe that it's necessary.

> To test this: Preference -> General -> Appearance. Switch from Light to Dark
> -> Should work fine without restart on Mac.
> 
> > The question was whether the
> > platform does anything helpful when the user manually imports preferences?
> 
> I almost never did import preference but I guess preference listeners are
> triggered during this import? 

Yes, preference changes trigger listeners.  But often the underlying assumption as that preference only change via the dialog such that there are no listeners.  That of course doesn't help when one imports preferences or uses Oomph's preference tasks.

> If yes, we could register a preference
> listener for theme changes, which should also help Oomph. Please confirm, in
> this case, I move this bug to platform.

If a restart really isn't required then yes, better that the platform listens for preferences changes such that manual imports or any other mechanism (Oomph's preference tasks) that change the value will have the same effect as changing it via the dialog.
Comment 6 Lars Vogel CLA 2019-11-14 05:26:53 EST
Thanks Ed, moving to platform. We should add a preference listener to the theme change.
Comment 7 Dani Megert CLA 2020-01-07 10:00:52 EST
Please set a target when you plan to work on this bug.
Comment 8 Lars Vogel CLA 2020-06-12 03:09:28 EDT
Andrew, something for you?
Comment 9 Andrew Obuchowicz CLA 2020-06-12 10:39:37 EDT
(In reply to Lars Vogel from comment #8)
> Andrew, something for you?

I'm not 100% sure I understand what needs to be done here.

Does a preference change event need to be fired when the theme changes?
Comment 10 Lars Vogel CLA 2020-06-12 10:50:23 EDT
(In reply to Andrew Obuchowicz from comment #9)
> (In reply to Lars Vogel from comment #8)
> > Andrew, something for you?
> 
> I'm not 100% sure I understand what needs to be done here.
> 
> Does a preference change event need to be fired when the theme changes?

Vice-versa, if the preference changes and theme event needs to get triggered.
Comment 11 Andrew Obuchowicz CLA 2020-07-02 15:11:50 EDT
(In reply to Lars Vogel from comment #10)
> (In reply to Andrew Obuchowicz from comment #9)
> > (In reply to Lars Vogel from comment #8)
> > > Andrew, something for you?
> > 
> > I'm not 100% sure I understand what needs to be done here.
> > 
> > Does a preference change event need to be fired when the theme changes?
> 
> Vice-versa, if the preference changes and theme event needs to get triggered.

If I understand the problem correctly, we want to make sure that when any preference change that would require re-theming, the theming engine should be triggered.

How would one determine which preference changes require a theme event to be triggered? I imagine one's that affect colors would require a theme change, many examples can be seen in https://github.com/AObuchow/Eclipse-Spectrum-Theme/blob/master/com.aobuchow.themes.spectrum/css/preference_styles.css#L142

But is there a way to filter for these types of preferences? I haven't actually looked into how these colore-related preference objects are implemented.
Comment 12 Lars Vogel CLA 2020-11-18 08:32:06 EST
Mass change to 4.19 M1, please update the target if you have other plans.
Comment 13 Alexander Kurtakov CLA 2021-01-07 03:07:11 EST
Mass move 4.19 M1 bugs to M3
Comment 14 Niraj Modi CLA 2021-02-19 07:59:08 EST
Removing milestone from 4.19 M3 to 4.19, please re-target accordingly.
Comment 15 Kalyan Prasad Tatavarthi CLA 2021-03-02 02:07:36 EST
Mass move out of 4.19
Comment 16 Kalyan Prasad Tatavarthi CLA 2021-06-04 00:54:23 EDT
Mass Move out of 4.20
Comment 17 Kalyan Prasad Tatavarthi CLA 2021-08-17 02:10:00 EDT
Mass Move out of 4.21
Comment 18 Kalyan Prasad Tatavarthi CLA 2021-11-19 01:02:27 EST
Mass Move out of 4.22