Community
Participate
Working Groups
I currently am using the TableTree that has a 5-level deep tree. I use a ViewerSorter to sort the elements based on simple compare routines. However, when I expand and collapse any particular set of nodes to arbitary depths and then sort on the column the nodes that I selected to be expanded/collapsed all collapse up. Note that the order of the items in the tree after this occurs is correctly sorted. Example: A) here is an tree that is fully expanded (by the user's mouse clicks) - B + B(1) - A - A(1) - A(1)(1) + A(1)(3) + A(1)(2) B) now I sort on a column and I get - A + A(1) - B + B(1) Notice that the order is correctly sorted (A before B) and the child elements of A(1) are now all collpased when they were explicitly expanded by the user before sorting. Here is an example of what I'm doing (I'm hoping you can see that I'm not explicitly controlling the expanding/collapsing of the nodes. I simply refresh the view): 1) I create a selection listener for the column to sort on _column.addSelectionListener(new SelectionListener() { public void widgetSelected(SelectionEvent e) { if (e.widget instanceof TreeColumn) { _columnSelection = getIndexOfSelectedColumn(); _treeViewer.getControl.setRedraw(false); _treeViewer.refresh(); _treeViewer.getControl.setRedraw(true); } public void widgetDefaultSelected(SelectionEvent e) { widgetSelected(e); } }); 2) My ViewerSorter class is coded similar to: _treeViewer.setSorter(new ViewerSorter() { public int compare(Viewer viewer, Object e1, Object e2) { switch (_columnSelection) { case 1: return compareStrings(e1,e2); case 2: return compareInts(e1,e2); } return 0; });
TreeViewer is a JFace component - moving to the UI team.
I can give this a try for 3.2 M4. Since this seems to be a major problem for you, would you be able to contribute a JUnit test for it?
I don't think that will be a problem. I think the first thing you should try is to recreate my problem with the current TreeViewer in Eclipse 3.1 (to verify that this is indeed a bug). If it is then I'll work on a JUnit.
Note that TableTree is deprecated. You should probably first verify that the same problem occurs with a Tree and TreeColumns.
You will also want to look at the TreeEditor and TreeEditorImpl (as these help construct the TreeViewer as a Viewer. I suspect the problem isn't in the TreeColumns class (just guessing) because this is a sorting problem of the tree nodes would logically be in the component that executes the reordering of the tree nodes. I suspect its in Tree or the TreeEditor.
Did some more digging and here is what I found using my initial example: - B + B(1) - A - A(1) - A(1)(1) + A(1)(3) + A(1)(2) When you click on a column to sort, my sorter class (which is an extension of ViewerSorter) has a compare() method that is only being called for the "root" elements in my tree - that is "A" and "B" above. During this same sorting session, the compare() method is not being called for any "child nodes". That is there is no call being made to compare() for "A(1)(1)" and "A(1)(2)" during the same sorting session. I think the ViewerSorter should be handling recursively calling the compare() method for child nodes after the root nodes in the tree are sorted.
I would like to elaborate and say that compare() is called for the child nodes when you click the "expand" button on the tree, however it is not called for the child nodes with you click on the column for sorting. In addition, this may also explaing why the tree collapses itself back up to the root level showing only 1 immediate child level and not any deeper (refer to the initial example I posted). I leave it up to you to verify.
This is still a bug in the current code (TreeViewer). Moving to M5 for fixing. Ashish, a test case would be very much appreciated, preferably in the form of a new method for AbstractTreeViewerTest.
I've taken a look at the AbstractTreeViewerTest class where you keep your current tests for this widget. However, I find if very difficult and complex to write a test case that sorts a sufficient sample data and verify that the sorting has worked correctly. The easiest way to verify the changes is to load a sample data set into the table then sort by clicking a column and visually verifying that the sort has worked. I say its quite complex to do this programmatically in the Junit test because the sample data set could be 5-8 levels deep. In addition, there are known problems with GTK to read values back out from the table during the verification process. A sufficient sample data set includes a tree that is 5-8 levels deep where the TreeViewer has 4-5 columns of data across these 5-8 levels in the tree. A smaller example of what I'm talking about is shown below: Column 1 Column 2 Column 3 Column 4 - B 16.00 8.00 4.000 + B(1) 8.00 4.00 2.000 + B(1) 8.00 4.00 2.000 - A 16.00 8.00 4.000 - A(1) 16.00 8.00 4.000 - A(1)(1) 8.00 4.00 2.000 + A(1)(3) 4.00 2.00 1.000 + A(1)(3) 4.00 2.00 1.000 + A(1)(2) 8.00 4.00 2.000 Notice that the values (in columns 2-4) of the child elements add up to the values(in columns 2-4) in the parent elements. Sorting on columns 2-4 should order the tree elements appropriately and preserve the tree structure (that is sort the parent tree element first then its children). Therefore, I don't a Junit test is the best way to verify this defect. I think one must manually do this. My suggestion is for you to actually use the TreeViewer and create a testing application that loads the data and sorts it - and visually verify it. Does anyone have opinions/comments on this? I have an application with valid data samples ready that prove the sorting to fail, so if you develop a patch that I can load into the workbench, I'd be glad to tell you if the fixes work or not.
Finally, we figured out what causes the problem: When TreeViewer does a refresh(), it reuses the existing TreeItems where possible, but it also disposes of invisible tree items that might exist as children of a collapsed node. Expansion state is only remembered for children of one parent while its children are being recomputed. As a result, there are cases where the expansion state of tree items deeper in the tree are lost. We need to remember the complete expansion state to fix this bug. One way of doing this would be to change preservingSelection() to preservingSelectionAndExpansion().
This issue is now fixed. For the record, there was no problem with the actual TreeViewer implementation. I wrote a test called TreeViewerAdvancedTest in org.eclipse.ui.tests that goes to verify that the sorting capability for deep trees works (Boris may commit this if he feels its necessary). The problem was in my code that was using the TreeViewer class. The problem of sorting was fixed using two fixes: 1) Before the TreeViewer.refresh() was called when the sorting process starts, the expanded elements were cached, and restored following the refresh: Object[] expandedElements = _treeViewer.getExpandedElements(); _treeViewer.refresh(); _treeViewer.setExpandedElements(expandedElements); 2) The data objects being stored for each TreeItem (TreeItem.setData()) did not have proper hashCode() methods implemented. Therefore, when the sorting occured the TreeItem nodes that were expanded before the sorting couldn't properly be found after the sorting. Both of these fixes failed in the TreeViewer's attempt to retaun the visual display of the expanded elements would fail and all the tree nodes would collapse on itself.
I'm reopening this because I think that TreeViewer should try to retain the expanded state of elements when refreshing. In other words, clients should not have to remember the expanded state before calling refresh() in order to restore it later. I'm downgrading to normal, and removing the target milestone because this is only a problem in deep trees that are partially expanded, and a workaround exists.
*** Bug 64140 has been marked as a duplicate of this bug. ***
> I'm downgrading to normal, and removing the target milestone because this is > only a problem in deep trees that are partially expanded, and a workaround > exists. A tree with only 3 levels is considered deep? This problem affects team synchronize views very often, since incoming and outgoing changes are constantly changing. If one uses the workaround, is it also necessary to disable redraw to prevent the platform's native animation?
(In reply to comment #14) > If one uses the workaround, is it also necessary to disable redraw to prevent > the platform's native animation? Good question. I don't know. I agree that reusing tree items creates problems. Obviously, it is too late to do anything about this in 3.3 given how late we are in the development cycle. I can mark this as 3.4 to make sure it appears on my radar again but cannot promise that we will have the time to fix it.
Why wouldn't this be a candidate for maintenance?
(In reply to comment #16) > Why wouldn't this be a candidate for maintenance? I am reluctant to fix this in a maintenance release because we only do limited testing for them, and changing this seems non-trivial to me and might even require new API.
*** Bug 35586 has been marked as a duplicate of this bug. ***
Created attachment 83448 [details] Sample to demonstrate the collapsing of nodes on a sort Demonstrates a sorting issue in the viewer: 1. Expand all elements: Schema DB1 DB2 T1 T2 C1 C2 C3 2. Sort by the Status column Schema DB1 DB2 T1 T2 C2 <-- These 2 were sorted correctly C1 <-- C3 3. Sort again by the Status column - T2 gets collapsed Schema DB2 T1 T2 <-T2 is now collapsed DB1
(In reply to comment #11) >I wrote a test called TreeViewerAdvancedTest in > org.eclipse.ui.tests that goes to verify that the sorting capability for deep > trees works (Boris may commit this if he feels its necessary). The problem was I'm not seeing this test class in org.eclipse.ui.tests. Is their a particular branch or version that this is in?
(In reply to comment #20) > (In reply to comment #11) > >I wrote a test called TreeViewerAdvancedTest > >... > I'm not seeing this test class in org.eclipse.ui.tests. Is their a particular > branch or version that this is in? No, this test was never contributed. Ashish, if you want to contribute that test it needs to be attached to a bugzilla.
*** Bug 227800 has been marked as a duplicate of this bug. ***
Mass update - removing 3.4 target. This was one of the bugs I marked for investigation (and potential fixing) in 3.4 but I ran out of time. Please ping on the bug if fixing it would be really important for 3.4, and does not require API changes or feature work.
We have run into this bug and I am not sure if I have a reasonable workaround for it. We extend org.eclipse.ui.navigator.navigatorContent as part of the common navigation framework and extending the Eclipse project explorer. When we call the viewer.refresh, sometimes the tree collapses. Weird, because it does not all the time. So, I tried to use the workaround suggested in this ticket and put getExpandedElements and setExpandedElements around the viewer.refresh call. But discovered that this technique does not work. Our code is told (by another subsystem that we do not control) that a node has changed and this node is the grandparent of the node that has really changed (removed/added). See example below. If our code was told to change the parent of the node that has changed then I think this workaround would work. For example, when the tree is expanded like this: A B C x y and x is removed, our code is told that B has changed. Hence, we refresh B. This call to viewer.refresh collapses C. Even if we used the workaround mentioned, we would only save and restore the expansion state of B, not C. Any ideas on a workaround for this case? If not, can we have this bug fixed soon?
(In reply to comment #24) > ... Even if we used the workaround > mentioned, we would only save and restore the expansion state of B, not C. I don't understand. treeViewer.getExpandedElements() returns all expanded elements, regardless how deep. Similarly, treeViewer.setExpandedElements() will expand all the elements you pass to it. There are cases when setExpandedElements() does not succeed, but these can be avoided by properly implementing the content provider's getParent() method.
Created attachment 123785 [details] potential solution (untested) Would something along the lines of this patch help?
Ok, I'll subclass AbstractTreeViewer and do what you are suggesting. Thanks. Back to my grandparent theory. I misunderstood the getExpandedElements method comments. I take back what I said that the workaround would not work in the grandparent case. But, it really did not work for me after I wrapped the refresh call with the getExpandedElements and setExpandedElements calls. The problem is the same when I wrapped them or when I do not. Sometimes it collapses the tree and sometimes it does not!! Using the debugger I can keep stepping through the code that send the events and see it sometimes collapse and sometimes not. I set breakpoints in certain code in the TreeViewer and its parents that I thought would do the collapsing but could never see when and why it decided to collapse sometimes. Weird. What could cause such a behaviour?
(In reply to comment #27) > What could cause such a behaviour? Check your implementation of ITreeContentProvider.getParent() (and the implementation of getParent() for all other participating tree content providers, in the Common Navigator case).
Hitesh is now responsible for watching bugs in the [Viewers] component area.
Created attachment 164351 [details] Simple snippet to reproduce the tree viewer bug This is a simple snippet to reproduce the bug. It creates a tree viewer with sample expanded content. When pressing a key an item is added and a different item in the tree collapses when the tree is refreshed. Note that the item stays expanded when the new item is added at the end of the items (see comment in code). I hope this example is useful.
I am getting exactly the same problem described by the comment 30. When a tree has fully expanded node, and a new node gets added above it, after refresh this expanded node is collapsed. A node remains expanded if the new node appears below it. I am using Eclipse 4.5 When debugging a working refresh scenario AbstractTreeViewer#updateChildren() successfully finds all the items for a top node, i.e getChildren() returns the number of items as expected. In a non working case, the above call only returns 1 child. When digging deeper, it turned out that in Tree#getItems() only loops the following code once while (hItem != 0) { hItem = OS.SendMessage (handle, OS.TVM_GETNEXTITEM, OS.TVGN_NEXT, hItem); count++; } It's all OS related code, so I've no clue what goes on, but after the first loop, hItem becomes 0 and only one TreeItem gets returned. Any workaround?
I am getting exactly the same problem described by the comment 31. When a tree has fully expanded node, and a new node gets added above it, after refresh this expanded node is collapsed. A node remains expanded if the new node appears below it. I am using Eclipse 4.6.1. Has someone a workaroud for it?
I tried the Workaround proposed by Boris. It works fine. But with big structures I have a performance Problem. I am playing with structures, which have more than 30.000 items. With such structures the execution of setExpandMethod can dauer more than 5 seconds. A refresh which dauer 6s instead of 1s is not really good :-( Has anyone a better idea?
I am running into this problem in the Project Explorer when CDT’s “Searching for Binaries” job completes and adds the “Binaries” entry at the top of a project, and it’s very annoying. I have a simple patch to Snippet002TreeViewer.java that demonstrates the problem, but I think there’s no need for that anymore as Dirk’s snippet does the same. Boris’ patch still applies in R4_7_maintenance and solves the Project Explorer issue for me. Should I push it to Gerrit?
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.
