Bugzilla – Full Text Bug Listing
| Summary: | [Viewers] TableViewer selecting items when it shouldn't | ||
|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Jared Burns <jared_burns> |
| Component: | UI | Assignee: | Platform-UI-Inbox <Platform-UI-Inbox> |
| Status: | RESOLVED WONTFIX | QA Contact: | |
| Severity: | enhancement | ||
| Priority: | P4 | CC: | heath.borders, Tod_Creasey |
| Version: | 2.1 | ||
| Target Milestone: | --- | ||
| Hardware: | Other | ||
| OS: | All | ||
| Whiteboard: | |||
|
Description
Jared Burns
The workaround is to forcibly clear the selection every time the view is refreshed and reset the selection to the correct item. The viewers generally don't handle multiple equal items well. In this case, the viewer tries to preserve selection across the refresh by remembering the selected model elements and re-selecting them after. The refresh may completely scramble the SWT TableItems (e.g. changing sort order), so it has to remember the selection in terms of model elements, not SWT items. But this means that it loses the knowledge that it's actually the second A that's selected, not the first (or more accurately, it never had this knowledge in the first place). Fixing this properly is a major change to the viewers, which will not happen for M5 (see below for details). Is it possible for you to call add() instead of refresh() in this case? That would avoid the problem entirely. You could also try subclassing the viewer and overriding preservingSelection() to do nothing other than run the runnable. But this will cause the selection to be incorrect if the order of elements changes. Alternatively, you could have it check the selection for any change (using getSelection) before calling setSelectionToWidget. This will work for the case where the elements and their order hasn't changed (refresh reuses TableItems rather than recreating them, so, as far as SWT is concerned, the selection hasn't changed). But this won't solve the problem in general (e.g. if C is inserted before either A), and it's not something I want to add at this point to StructuredViewer, since getting the selection can be quite costly (O(N) for trees in Windows). To handle multiple equal elements properly in the viewers would require two major design changes: 1. Support a mapping from one element to multiple SWT items in findItem. This includes being able to map from an element to multiple items in the elementMap. 2. Avoid reusing SWT items. This has downsides in terms of performance and UI flashiness. In my case, I actually know that the order of the items has changed - it's in our implementation of the "move up"/"move down" actions. The "add" workaround won't work for us, because we're not adding elements. Our code changes the order of the elements in our content provider, then calls refresh() so the viewer picks up the ordering change, and then manually sets the selection to what it should be with the new ordering. I could have sworn that this bug didn't exist when I first wrote this code to handle the duplicate item case (a month ago?). Is this code new? For M5, I think we'll just go with the ugly workaround (clear the selection after the refresh and then set it ourselves). For 2.1, we'll probably go with the viewer subclassing. There was a change recently in how viewers compare elements. You can now set an element comparer to change how equals and hashCode are done. See IElementComparer. However, this should not affect how viewers refresh (unless you set one). As a sanity check, I went back and tested this behavior on the 20021218 build. Turns out I'm not sane as the problem exists in that build too. So the code that's affecting us isn't the new code. There are no plans for the UI team to work on this defect until higher priority items are addressed. If you would like to work on this defect, please let us know on the platform-ui-dev mailing list. There are currently no plans to work on this feature |