Community
Participate
Working Groups
- happens in Table and Tree, but not in List - run the snippet below and click on any item in the Table -> this selects the item and deselects the others, but sends two Selection events instead of the expected one Selection event -> verified that win32 and gtk only send one in this case, didn't try carbon public static void main(String[] args) { final Display display = new Display(); Shell shell = new Shell(display); shell.setBounds(200, 200, 200, 200); shell.setLayout(new FillLayout()); final Table table = new Table(shell, SWT.MULTI); for (int i = 0; i < 9; i++) { new TableItem(table, SWT.NONE).setText("hi " + i); } shell.open(); table.selectAll(); table.addListener(SWT.Selection, new Listener() { public void handleEvent(Event event) { System.out.println("selection"); } }); while (!shell.isDisposed()) { if (!display.readAndDispatch()) display.sleep(); } display.dispose(); }
The first selection event happens when the mouseDown is processed. That was added in bug 279103. The second selection event happens because clearing the remaining selected items changes the selection, so we send the event in response to the Cocoa notification. I think we just need to be smarter in mouseDownSuper; it sends a Selection event when the only item selected was clicked on without modifiers, but if other rows are selected we can just wait for the tableViewSelectionDidChange notification.
Created attachment 172081 [details] Bigger fix Fix for this and bug 317051. In Table and Tree, make sure only one item is selected before sending selection event during mouse down processing.
All set. Grant, please review for 3.6.1.
The patch looks generally good. The only part that's not ideal is the additions of the "&& widget.selectedRowIndexes().count() == 1". It would be better if there was some way to prevent the second Selection event from being sent instead of preventing the first, since the current fix makes the event ordering be "mouse down - mouse up - selection" instead of "mouse down - selection - mouse up". Is this (reasonably) possible?
(In reply to comment #4) > The patch looks generally good. The only part that's not ideal is the > additions of the "&& widget.selectedRowIndexes().count() == 1". It would be > better if there was some way to prevent the second Selection event from being > sent instead of preventing the first, since the current fix makes the event > ordering be "mouse down - mouse up - selection" instead of "mouse down - > selection - mouse up". Is this (reasonably) possible? Let me see what I can do. I think we would only want to ignore one selection notification at a time, so maybe I can set ignoreSelect if the selection event was sent during mouse handling and then clear the flag in tableViewSelectionDidChange.
Created attachment 172259 [details] One more fix This puts the events back in the right order. Looks good based on what I'm seeing in ControlExample.
The latest patch seems to work well, and I couldn't find any cases broken by it. I would just suggest adding a comment to the mouseDownSuper changes since it may not be clear why the selectedRowCount > 1 case is getting special treatment, and to indicate where ignoreSelect is later reset to false.
(In reply to comment #7) > The latest patch seems to work well, and I couldn't find any cases broken by > it. I would just suggest adding a comment to the mouseDownSuper changes since > it may not be clear why the selectedRowCount > 1 case is getting special > treatment, and to indicate where ignoreSelect is later reset to false. Okay, will do. Now I just need to wait for 3.6.1 to open up.
Created attachment 172925 [details] Final patch Same as previous patch with extra comments.
Fixed in 3.6.1 and HEAD > 20100628.
We need to revisit this fix. The change in Table.tableViewSelectionDidChange() is not correct. We must not send SWT.Selection events when Table.deselect(), etc. methods are called. With the change only the first deselect is ignored. Run the snippet and press any key. You get two selection events. public static void main(String[] args) { try { final Display display = new Display(); Shell shell = new Shell(display); shell.setBounds(200, 200, 200, 200); shell.setLayout(new FillLayout()); final Table table = new Table(shell, SWT.MULTI); for (int i = 0; i < 9; i++) { new TableItem(table, SWT.NONE).setText("hi " + i); } shell.open(); table.selectAll(); table.addListener(SWT.Selection, new Listener() { public void handleEvent(Event event) { System.out.println("selection"); } }); table.addListener(SWT.KeyDown, new Listener() { public void handleEvent(Event event) { System.out.println("before"); table.deselect(new int[]{2, 3}); System.out.println("after"); } }); while (!shell.isDisposed()) { if (!display.readAndDispatch()) display.sleep(); } display.dispose(); } catch (Throwable e) { e.printStackTrace(); } }
(In reply to comment #11) > We need to revisit this fix. The change in Table.tableViewSelectionDidChange() > is not correct. We must not send SWT.Selection events when Table.deselect(), > etc. methods are called. With the change only the first deselect is ignored. > > Run the snippet and press any key. You get two selection events. I only get two selection events if I hit an up or down arrow. Left or right or any other only gives one selection event, but I agree that none should be sent in that case.
Created attachment 173050 [details] One more fix Reset ignoreSelect on a click or key down. Then we can safely ignore the selection change after a click on a selected item. Also, I realized List should look more like Table in this area -- this and the related bugs (317051) have the same problems if you use a List instead of a Table.
One more review, since it's for 3.6.1. I'll put it into HEAD in any event.
List.sendDoubleSelection() is sending SWT.Selection instead of SWT.DefaultSelection.
(In reply to comment #15) > List.sendDoubleSelection() is sending SWT.Selection instead of SWT.DefaultSelection. Oops. Got too aggressive with the refactoring. Fixed in HEAD.
+1 for 3.6.1 (patch + change made to HEAD)
Fixed in 3.6 branch and HEAD > 20100706.
