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

Bug 333541

Summary: [preferences] "Show whitespace characters" gets toggled when "whitespace characters" dialog is opened
Product: [Eclipse Project] Platform Reporter: Mark McLoughlin <markmc>
Component: TextAssignee: Deepak Azad <deepakazad>
Status: RESOLVED FIXED QA Contact:
Severity: normal    
Priority: P3 CC: daniel_megert, deepakazad, lshanmug
Version: 3.7Flags: daniel_megert: review+
Target Milestone: 3.7 M5   
Hardware: PC   
OS: Linux   
Whiteboard:
Attachments:
Description Flags
fix
daniel_megert: review-
fix
daniel_megert: review-
fix none

Description Mark McLoughlin CLA 2011-01-05 04:45:39 EST 
Build Identifier: 20101216-1529

Using eclipse-java-indigo-M4-linux-gtk

When you click on the "whitespace characters" link to open the "Show Whitespace Characters" dialog, then checkbox is always toggled

For the use case of someone enabling the option, that works fine - click the link, tweak the options in the dialog, close and the option is enabled

However, if the option is already enabled and you just want to tweak the options, it gets really annoying - click the link, tweak the options, close and notice the option is now disabled

Suggest that the the checkbox should never be toggled from on to off by clicking on this link

For reference, the option was added by bug #257313

Reproducible: Always

Steps to Reproduce:
1. In Preferences -> General -> Text Editors
2. Click the "whitespace characters" link and get the "Show Whitespace Characters" dialog
3. Notice the checkbox has been toggled
Comment 1 Deepak Azad CLA 2011-01-05 05:41:14 EST 
I can reproduce on Linux, though it works fine on Windows. I will take a look.
Comment 2 Deepak Azad CLA 2011-01-20 04:55:59 EST 
Created attachment 187171 [details]
fix

The focus listener was behaving differently on windows and linux. However, it was not really needed. 

The real problem was that there are 2 selection listeners - one is used to open a dialog and one is used to set linkSelected[0] to true. Opening a dialog takes some time and it delays the second selection listener. I have changed the order in which the selection listeners are added so that linkSelected[0] is set to true before the mouseUp event.
Comment 3 Deepak Azad CLA 2011-01-20 04:58:15 EST 
Dani, if it looks ok to you please commit the patch.

Lakshmi, thanks for the help!
Comment 4 Dani Megert CLA 2011-01-20 11:42:02 EST 
>  I have changed the order
> in which the selection listeners are added so that linkSelected[0] is set to
> true before the mouseUp event.
This is not a real fix as the order in which the added listeners are notified  isn't guaranteed via API.
Comment 5 Deepak Azad CLA 2011-01-21 01:56:00 EST 
Created attachment 187260 [details]
fix

Alright, in this patch I have removed the focus listener and then open the dialog in a job.

(Another option would be to open a bug with SWT - focus listener behaves one way on Windows and another way on Linux.)
Comment 6 Dani Megert CLA 2011-01-21 02:47:38 EST 
(In reply to comment #5)
> Created attachment 187260 [details] [diff]
> fix
> 
> Alright, in this patch I have removed the focus listener and then open the
> dialog in a job.
> 
> (Another option would be to open a bug with SWT - focus listener behaves one
> way on Windows and another way on Linux.)

Using a job here is overkill (really ;-). Just make sure your listeners are notified in the sequence you need (similar to your previous fix).
Comment 7 Deepak Azad CLA 2011-01-21 04:40:06 EST 
Created attachment 187263 [details]
fix

This one should be good.
Comment 8 Dani Megert CLA 2011-01-21 05:16:54 EST 
Patch is almost fine except for the useless 'widgetDefaultSelected' method. I've removed that and committed it to HEAD.

Please make sure to
- verify the fix on Linux
- check JDT and Text for similar pattern and provide a patch here (for Text)
Comment 9 Deepak Azad CLA 2011-01-21 06:12:04 EST 
Fixed in OptionsConfigurationBlock as well, and verified on Linux.
Comment 10 Mark McLoughlin CLA 2011-01-21 06:27:38 EST 
Thanks Deepak, it's great to see the bug report handled quickly!