Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 546140

Summary: Remove ability to set MRU via CSS
Product: [Eclipse Project] Platform Reporter: Lars Vogel <Lars.Vogel>
Component: UIAssignee: Lars Vogel <Lars.Vogel>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: daniel_megert, Lars.Vogel, loskutov, mauromol
Version: 4.11   
Target Milestone: 4.17 M3   
Hardware: PC   
OS: Linux   
See Also: https://git.eclipse.org/r/140097
https://bugs.eclipse.org/bugs/show_bug.cgi?id=388476
https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/166517
https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/166522
https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=0862444e616c785f5cbc5900e2025952cac1df0d
https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=60b49762aabb9eee263a4c968b8742c2ddcabd13
Whiteboard:
Bug Depends on:    
Bug Blocks: 563540    

Description Lars Vogel CLA 2019-04-05 05:33:40 EDT
Dark theme specifies 

swt-mru-visible: true;

IIRC Andrey added MRU as preference which has priority over CSS. So I think we should remove it from the CSS.
Comment 1 Lars Vogel CLA 2019-04-05 05:36:41 EDT
Actuall all themes specifies this. I create a change to remove it and hope that Andrey has time to respond. As I'm ignoring the existing of the MRU setting, I'm a bad judge if this change will affect MRU haters and lovers.
Comment 2 Eclipse Genie CLA 2019-04-05 05:37:21 EDT
New Gerrit change created: https://git.eclipse.org/r/140097
Comment 3 Dani Megert CLA 2019-04-05 09:53:54 EDT
(In reply to Lars Vogel from comment #1)
> Actuall all themes specifies this. I create a change to remove it and hope
> that Andrey has time to respond. As I'm ignoring the existing of the MRU
> setting, I'm a bad judge if this change will affect MRU haters and lovers.
How can you provide a change of which you do not know the impact?
Comment 4 Lars Vogel CLA 2019-04-05 10:46:58 EDT
(In reply to Dani Megert from comment #3)
> (In reply to Lars Vogel from comment #1)
> > Actuall all themes specifies this. I create a change to remove it and hope
> > that Andrey has time to respond. As I'm ignoring the existing of the MRU
> > setting, I'm a bad judge if this change will affect MRU haters and lovers.
> How can you provide a change of which you do not know the impact?

By asking the expert (Andrey) for his opinion.
Comment 5 Andrey Loskutov CLA 2019-04-05 10:52:25 EDT
(In reply to Lars Vogel from comment #4)
> 
> By asking the expert (Andrey) for his opinion.

I'm not expert in themes, I always have them disabled.
Comment 6 Lars Vogel CLA 2019-04-05 10:53:43 EDT
(In reply to Andrey Loskutov from comment #5)
> (In reply to Lars Vogel from comment #4)
> > 
> > By asking the expert (Andrey) for his opinion.
> 
> I'm not expert in themes, I always have them disabled.

Didn't you implement that the preference overrides the CSS setting?
Comment 7 Dani Megert CLA 2019-04-05 10:54:21 EDT
(In reply to Andrey Loskutov from comment #5)
> (In reply to Lars Vogel from comment #4)
> > 
> > By asking the expert (Andrey) for his opinion.
> 
> I'm not expert in themes, I always have them disabled.
Ha!
Comment 8 Lars Vogel CLA 2019-04-05 11:01:01 EDT
(In reply to Dani Megert from comment #7)
> (In reply to Andrey Loskutov from comment #5)
> > (In reply to Lars Vogel from comment #4)
> > > 
> > > By asking the expert (Andrey) for his opinion.
> > 
> > I'm not expert in themes, I always have them disabled.
> Ha!

Ha ha? Let's stay constructive
Comment 9 Andrey Loskutov CLA 2019-04-05 11:05:50 EDT
(In reply to Lars Vogel from comment #6)
> (In reply to Andrey Loskutov from comment #5)
> > (In reply to Lars Vogel from comment #4)
> > > 
> > > By asking the expert (Andrey) for his opinion.
> > 
> > I'm not expert in themes, I always have them disabled.
> 
> Didn't you implement that the preference overrides the CSS setting?

Right, I did this long time ago, see commit https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=ead0d552a9cadb8b02bab895e18c51a8c78a2235 for bug 388476. So by default those aren't relevant anymore, except user changes the default.
Comment 10 Lars Vogel CLA 2019-04-05 11:13:13 EDT
(In reply to Andrey Loskutov from comment #9)
> (In reply to Lars Vogel from comment #6)
> > (In reply to Andrey Loskutov from comment #5)
> > > (In reply to Lars Vogel from comment #4)
> > > > 
> > > > By asking the expert (Andrey) for his opinion.
> > > 
> > > I'm not expert in themes, I always have them disabled.
> > 
> > Didn't you implement that the preference overrides the CSS setting?
> 
> Right, I did this long time ago, see commit
> https://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/
> ?id=ead0d552a9cadb8b02bab895e18c51a8c78a2235 for bug 388476. So by default
> those aren't relevant anymore, except user changes the default.

Thanks Andrey, surprised that is already that long ago, feels like short term to me. 

I will have a closer look next week to see if we can safefy remove the CSS for the default. IIRC you argued that CSS is misuse for this case to which I agree. We could actually also style the preference via CSS if required.
Comment 11 Lars Vogel CLA 2019-07-05 11:13:31 EDT
No time for detailed testing for this trivial change.
Comment 12 Lars Vogel CLA 2019-11-06 12:04:17 EST
CSS engine is not API, so we should simplify remove the CSS handler. See CSSPropertyMruVisibleSWTHandler and its usage.
Comment 13 Lars Vogel CLA 2019-11-06 12:04:22 EST
CSS engine is not API, so we should simplify remove the CSS handler. See CSSPropertyMruVisibleSWTHandler and its usage.
Comment 14 Eclipse Genie CLA 2020-07-20 07:21:44 EDT
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/166517
Comment 15 Eclipse Genie CLA 2020-07-20 08:14:39 EDT
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/166522