Bug 563695

Summary:	Pressing cancel on the Apperance preference page triggers the CSS engine
Product:	[Eclipse Project] Platform	Reporter:	Lars Vogel <Lars.Vogel>
Component:	UI	Assignee:	Andrew Obuchowicz <aobuchow>
Status:	NEW ---	QA Contact:
Severity:	normal
Priority:	P3	CC:	aobuchow, Lars.Vogel, paul-eclipse
Version:	4.14
Target Milestone:	---
Hardware:	PC
OS:	Linux
URL:	The title bar is unnecessary sometimes when for example, detaching a Java Editor or Git staging View.
See Also:	https://git.eclipse.org/r/163840
Whiteboard:

Description Lars Vogel

2020-05-29 02:33:28 EDT

If you press cancel on the preference page, I'm surprised that we still trigger a CSS theming event. Maybe this can be avoided?

Comment 1 Lars Vogel

2020-05-29 02:34:15 EDT

Andrew something for you?

Comment 2 Andrew Obuchowicz

2020-05-29 06:30:04 EDT

+1 I've noticed this and it's annoying as a user. I'll have a look :)

Comment 3 Andrew Obuchowicz

2020-05-29 08:03:34 EDT

I looked into the git history, and it seems that the cancel button was made to trigger the CSS engine in Bug 317125.

I couldn't find an explanation as to why this was done. Personally, I see no reason to keep any theming logic in the cancel button (it should just close the preference dialog).

If there's no objection to this, we can merge a patch to remove this theming logic.

Comment 4 Lars Vogel

2020-05-29 08:06:45 EDT

+1

Comment 5 Eclipse Genie

2020-05-29 08:07:34 EDT

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

Comment 6 Paul Pazderski

2020-05-29 08:17:01 EDT

(In reply to Andrew Obuchowicz from comment #3)
> I looked into the git history, and it seems that the cancel button was made
> to trigger the CSS engine in Bug 317125.
> 
> I couldn't find an explanation as to why this was done. Personally, I see no
> reason to keep any theming logic in the cancel button (it should just close
> the preference dialog).
> 
> If there's no objection to this, we can merge a patch to remove this theming
> logic.

I had to use theming preferences quite often recently (mostly involuntary) and noticed immediate changes in rendering from the "Color and Font Theme" drop down without pressing apply. Maybe this call on cancel was to revert such changes.

Comment 7 Andrew Obuchowicz

2020-05-29 08:49:18 EDT

> I had to use theming preferences quite often recently (mostly involuntary)
> and noticed immediate changes in rendering from the "Color and Font Theme"
> drop down without pressing apply. Maybe this call on cancel was to revert
> such changes.

I think you solved the mystery, Paul :D

In this case, this patch should also deal with preventing "eager" changes to the css engine. IMO changes should only be applied when the Apply or Apply and Close buttons are preseed.

Comment 8 Paul Pazderski

2020-05-29 09:11:27 EDT

(In reply to Andrew Obuchowicz from comment #7)
> IMO changes should only be applied when the Apply or Apply
> and Close buttons are preseed.

I agree on that part that the drop-down is misbehaving.

Comment 9 Andrew Obuchowicz

2020-05-29 15:05:00 EDT

(In reply to Paul Pazderski from comment #8)
> (In reply to Andrew Obuchowicz from comment #7)
> > IMO changes should only be applied when the Apply or Apply
> > and Close buttons are preseed.
> 
> I agree on that part that the drop-down is misbehaving.

The latest gerrit fixes this :)