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

Bug 355200

Summary: Table and Tree selection wrong during widgetSelected when selecting element of multiple selection
Product: [Eclipse Project] Platform Reporter: David Huber <david>
Component: SWTAssignee: Lakshmi P Shanmugam <lshanmug>
Status: VERIFIED FIXED QA Contact: Lakshmi P Shanmugam <lshanmug>
Severity: normal    
Priority: P3 CC: AndrewBachmann, curtis.windatt.public, daniel_megert, eclipse, harp, kevin, marcel, matthias.sohn, praveenasram, qforce, Silenio_Quarti
Version: 3.7   
Target Milestone: 4.3 M3   
Hardware: PC   
OS: Mac OS X   
Whiteboard:
Attachments:
Description Flags
Example code to verify the problem
none
patch
none
possible patch
none
patch-2 none

Description David Huber CLA 2011-08-19 05:40:28 EDT 
Build Identifier: 20110615-0604

In an table initialized with SWT.MULTI, when selecting a single element that was part of a multiple selection, will give a wrong selection during the change notifications (SelectionListener::widgetSelected).
Not sure if it is related, but the gui updates the selection only after the widgetSelected methods have been called. 

Reproducible: Always

Steps to Reproduce:
1. Run the attached code
2. Select two or more items in the table
3. without deselecting first, click on the last item of the selection
4. Message in the console shows the items selected at point 2
Comment 1 David Huber CLA 2011-08-19 05:42:09 EDT 
Created attachment 201772 [details]
Example code to verify the problem
Comment 2 David Huber CLA 2011-08-19 05:48:30 EDT 
*** Bug 354969 has been marked as a duplicate of this bug. ***
Comment 3 Kevin Sawicki CLA 2011-11-15 13:20:40 EST 
This seems quite severe since commands using selections will use elements that the user didn't intend to be used.
Comment 4 Lakshmi P Shanmugam CLA 2012-08-07 04:52:23 EDT 
*** Bug 379090 has been marked as a duplicate of this bug. ***
Comment 5 Lakshmi P Shanmugam CLA 2012-08-07 05:15:09 EDT 
*** Bug 343778 has been marked as a duplicate of this bug. ***
Comment 6 Thomas Singer CLA 2012-09-04 03:49:15 EDT 
IMHO, this is a very serious bug! Imagine a file manager where the user has 5 files selected. Then she selects one of these files and invokes Delete. All 5 files will be deleted.

Please raise the priority to Critical and solve it quickly! Thanks in advance.
Comment 7 Nam Quang Tran CLA 2012-09-04 10:10:34 EDT 
Possible workaround: On the Mac on which I tested this, Table.getSelection will return the new, "correct" selection about 500 ms after widgetSelected is called. That means if you want to get a useful selection from the table, you can wrap the code inside your widgetSelected method in a Display.timerExec call with a delay of more than 500 ms. (That also means Display.asyncExec won't work.)
Comment 8 Lakshmi P Shanmugam CLA 2012-09-05 06:52:39 EDT 
Created attachment 220726 [details]
patch

The problem is that the selection event is sent before the selection is updated in the view. So, calling getSelection() in the Selection listener returns the incorrect value. The patch deselects the items manually before the selection event is sent.
Silenio, can you please review the patch?
Comment 9 Silenio Quarti CLA 2012-09-05 15:17:39 EDT 
(In reply to comment #8)
> Created attachment 220726 [details]
> patch
> 
> The problem is that the selection event is sent before the selection is
> updated in the view. So, calling getSelection() in the Selection listener
> returns the incorrect value. The patch deselects the items manually before
> the selection event is sent.
> Silenio, can you please review the patch?

We cannot deselect the items in mouseDown(), otherwise we will not be able to drag and drop multiple items ever. Add this line to the sample, select multiple items and start dragging.

	DragSource dragSource = new DragSource(table, SWT.NONE);

The delayed deselection is a Mac behaviour. It happens in Finder too. I think what is wrong here is that we are not send a selection event when that happens.  Selection can never change without a SWT.Selection event.
Comment 10 Silenio Quarti CLA 2012-09-05 15:23:42 EDT 
Created attachment 220751 [details]
possible patch

Lakshmi, would this patch work?

It still sends the extra selection event when clicking in an item already selected (selection count=1). Otherwise, it sends the selection event when the items are deselected (after the cocoa timer expire).
Comment 11 Lakshmi P Shanmugam CLA 2012-09-06 06:35:46 EDT 
(In reply to comment #10)
> Created attachment 220751 [details]
> possible patch
> 
> Lakshmi, would this patch work?
> 
> It still sends the extra selection event when clicking in an item already
> selected (selection count=1). Otherwise, it sends the selection event when
> the items are deselected (after the cocoa timer expire).

Silenio, I tried the patch. This causes the Selection event to be sent after MouseUp. (and cause some old bugs like Bug 289483). 
I also noticed that the Selection event is not sent until I move the mouse or perform some other action.
Comment 12 Lakshmi P Shanmugam CLA 2012-09-06 06:46:40 EDT 
(In reply to comment #9)
> We cannot deselect the items in mouseDown(), otherwise we will not be able
> to drag and drop multiple items ever. Add this line to the sample, select
> multiple items and start dragging.
> 
> 	DragSource dragSource = new DragSource(table, SWT.NONE);
> 
Thanks Silenio, I tried this and can see the problem with deselecting items on mouseDown(). I'm thinking of doing this in mouseUp() instead, before sending the MouseUp event. This way the order of events can be maintained and Drag case can also be handled.
Comment 13 Lakshmi P Shanmugam CLA 2012-09-06 09:57:15 EDT 
Created attachment 220785 [details]
patch-2

patch based on above comments.
Comment 14 Silenio Quarti CLA 2012-09-18 10:00:16 EDT 
(In reply to comment #13)
> Created attachment 220785 [details]
> patch-2
> 
> patch based on above comments.

Patch looks good.
Comment 15 Lakshmi P Shanmugam CLA 2012-09-18 10:49:02 EDT 
Thanks Silenio! I'll commit this soon after M2.
Comment 16 Lakshmi P Shanmugam CLA 2012-09-24 07:28:19 EDT 
Pushed to master > http://git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=bc8036435837353efa5da8c19b6a3003dbb86045

Silenio, I noticed that an extra selection event is sent after MouseUp. I'm not sure where it is coming from though.
Comment 17 Lakshmi P Shanmugam CLA 2012-10-25 06:30:13 EDT 
Committed fix for the extra selection event to master > 
git.eclipse.org/c/platform/eclipse.platform.swt.git/commit/?id=81fc4fbb51e1206ed8a526050d3bdcad1a2f0496
Comment 18 Lakshmi P Shanmugam CLA 2012-10-31 05:24:26 EDT 
verified in I20121030-2000