Bugzilla – Full Text Bug Listing
| Summary: | [Viewers] TreeViewer.setSelection(ITreeSelection) can select wrong item if multiple leaf nodes hold the same data | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | [Eclipse Project] Platform | Reporter: | Adrian Price <adrianp.quatinus> | ||||||
| Component: | UI | Assignee: | Platform UI Triaged <platform-ui-triaged> | ||||||
| Status: | CLOSED WONTFIX | QA Contact: | |||||||
| Severity: | normal | ||||||||
| Priority: | P3 | CC: | billfenlason, remy.suen, truesoft.romania | ||||||
| Version: | 3.4.2 | ||||||||
| Target Milestone: | --- | ||||||||
| Hardware: | PC | ||||||||
| OS: | Windows XP | ||||||||
| Whiteboard: | stalebug | ||||||||
| Attachments: |
|
||||||||
Do you have sample code to reproduce this problem? Does the problem occur on Eclipse 3.6.1? http://download.eclipse.org/eclipse/downloads/drops/R-3.6.1-201009090800/index.php I'll see if I can put a simple reproducer together. It looks like Eclipse-3.6.1 AbstractTreeViewer.isSameSelection(List, Item[]) uses identical logic so I'd reckon that this is still a problem. Created attachment 185382 [details]
Reproducer
Based on JFace Snippet002TreeViewer.
Just a question: Is it not true that the model has to be hierarchical in nature, and that getParent must return only one node? If that is the case, one of the two paths must be invalid, because the parent of X can not be both B and C? Isn't it true that just because a path can be specified does not mean that it is valid? I notice that in the attachment code the node parents are not set. Don't they have to be? This is a bit like multiple inheritance, and I didn't think it was allowed. Created attachment 231226 [details]
Corrected Reproducer
In reply to comment #4: - Re. the reproducer: good catch - for some reason I had assumed that JFace TreeNodes behaved like EMF, where parent.setChildren(childNodes) implicitly calls child.setParent(parent) on each child and vice-versa; apparently it does not, but the attached corrected reproducer still exhibits the same problem. - It is *not* true that the underlying model has to be hierarchical in nature, but clearly for any given TreeNode, getParent() can only return one node. That is not the same as saying that two tree nodes may not hold the same data (which they may). Were this not the case, and the underlying model had to be strictly hierarchical (i.e., only containment parent:child references), the API would not need the TreePath class. I was thinking that the widget item data was directly linked to the model element, but now I understand that is not the case. While I was checking the code related to a bug I submitted (408380), I seem to remember seeing a comment that the code was dependent on the getParent function of the model (implying hierarchical nature). I may be mistaken about what I read. Allowing the same data for different paths implies that the map should (in effect) use the path, rather than the path element, as the key to locate the corresponding widget. I'll be interested to go back and see how it is implemented and how the map is maintained when model elements are added or deleted, particularly when an element with children is involved. Since I'm dealing with large trees (depth levels of 50 or more) I'm also concerned about performance. Perhaps the difficulty in mapping a tree path to the corresponding widget is related to my problem. It involves using the TreeSelection returned by getSelection in a following setSelection. This bug hasn't had any activity in quite some time. Maybe the problem got resolved, was a duplicate of something else, or became less pressing for some reason - or maybe it's still relevant but just hasn't been looked at yet. As such, we're closing this bug. If you have further information on the current state of the bug, please add it and reopen this bug. The information can be, for example, that the problem still occurs, that you still want the feature, that more information is needed, or that the bug is (for whatever reason) no longer relevant. -- The automated Eclipse Genie. |
Build Identifier: 3.4.2 If you have a tree viewer that contains duplicate elements on different tree paths, passing a ITreeSelection to TreeViewer.setSelection(ISelection) often fails to select the correct items. This is because TreeViewer.isSameSelection(List, Item) tests only for matching data but fails to take account of fact that 'items' and 'current' refer to *different* tree items! Thus, a previously selected item will remain selected if its data equals that of the item identified by the tree path. If the current selection does not have the same data as the new selection, things work correctly. Reproducible: Always Steps to Reproduce: For example: A B X <-- previously selected C X <-- cannot select this item using its tree path In other words setSelection(TreeSelection[A,C,X]) will actually leave TreePath[A,B,X] selected, which is not the correct item.