Bug 563695 - Pressing cancel on the Apperance preference page triggers the CSS engine
Summary: Pressing cancel on the Apperance preference page triggers the CSS engine
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 4.14   Edit
Hardware: PC Linux
: P3 normal (vote)
Target Milestone: ---   Edit
Assignee: Andrew Obuchowicz CLA
QA Contact:
URL: The title bar is unnecessary sometime...
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-29 02:33 EDT by Lars Vogel CLA
Modified: 2020-05-29 15:05 EDT (History)
3 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 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 CLA 2020-05-29 02:34:15 EDT
Andrew something for you?
Comment 2 Andrew Obuchowicz CLA 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 CLA 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 CLA 2020-05-29 08:06:45 EDT
+1
Comment 5 Eclipse Genie CLA 2020-05-29 08:07:34 EDT
New Gerrit change created: https://git.eclipse.org/r/163840
Comment 6 Paul Pazderski CLA 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 CLA 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 CLA 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 CLA 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 :)