Community
Participate
Working Groups
Build ID: I20061214-1445 This is with 3.3M4 using a TreeViewer with SWT.VIRTUAL and a ILazyTreePathContentProvider. The scenario is as follows: There are a couple of root elements in the tree that can be sorted (in the model). After the sorting, the viewer is refreshed. Observed: In this situation the selection is often not preserved. If there are expanded elements in the tree, blank children are visible and the child count doesn't seem to be correct for some elements, which in turn can lead to AIOOBEs in the content provider. Steps To Reproduce: I have attached a plug-in that allows to reproduce the behaviour. Open the SampleView and follow these steps: For the preservingSelection part: 1. Select the first element in the tree 2. Press the Sort action repeatedly -> The selection will go away For the refresh part: 1. Expand some elements in the tree 2. Press Sort -> There are blank children More information: The problem both for the preservingSelection and the refresh artifacts seems to be that lazily updating single items leaves the tree temporarily in a state where multiple equal elements exist on the same level. An example to illustrate: 1. Original state: [element 1] [element 2] [element 3] 2. Reverse sort order of the model elements and refresh the viewer 3. Callback to update the first element leads to the following state [element 3] [element 2] [element 3] In this situation, the selection cannot be restored as [element 1] is temporarily missing.
Created attachment 58104 [details] Plugin-in with a sample view demonstrating the problem
Are we doing anything for 3.3 ?
Darin, could this be related to bug 173898?
Yes, I think this is related to bug 173898. As part of my patch to that problem, I overrode replace(...) in the tree viewer to "clearAll" children when an item gets mapped to a new element - that way we are asked to refresh the children again for that element.
I have a fix for the refresh case (no blank children anymore), but preserving the selection (and expansion state) is difficult, if not impossible for virtual trees. We should discuss this when you have time.
(In reply to comment #0) > ...which in turn can lead to AIOOBEs in the content provider. The content provider should tolerate this - see the Javadoc: "If the content provider knows the child element for the given parent at this index, it should respond by calling {@link TreeViewer#replace(Object, int, Object)}" I.e. if the content provider does not have anything at that index, it should not respond, and certainly not throw exceptions.
Created attachment 63551 [details] work in progress (tests don't pass yet)
Created attachment 63669 [details] updated patch (still got an endless loop in the test) Isn't it fun?
Created attachment 63923 [details] updated patch, to be tested on GTK and MacOS The endless loop was a problem in the tests. With TreeViewer and SWT.VIRTUAL, you have to be very careful not to put two equal elements under the same parent.
Created attachment 63926 [details] updated patch, tests are green on MacOS
Created attachment 63930 [details] Standalone snippet for manual testing
The tests pass on GTK, but by running the snippet, I was able to crash SWT, see bug 182598.
The tests pass on Windows XP too. Darin, Patrick, could you give this patch a try please?
The snippet works on the Mac and on Windows XP.
The debugger still works with the patch. It doesn't fix the debugger's problem noted in bug 182008, but that could be due to debug specific code.
(Note, with this patch, I can remove my workaround to clear items when they are replaced in the tree).
Created attachment 64058 [details] latest patch Do not call clearAll() from setChildCount() any more.
Created attachment 64333 [details] updated patch, hopefully no longer causing GPs
Today's patch tested on the Mac (tests pass, snippet works). Found bug 183254.
Tested on GTK, tests pass, snippet works, found bug 183267.
Patch released >20070419.
This fix is causing expanded nodes in the debug viewers to flash on each refresh. The viewer is supressing setData callbacks on expanded nodes which does not allow us to preserve lables. See bug 183463. We can workaround it, but the workaround is not optimal (requires copying private methods from TreeViewer). Perhaps the viewer could preserve labels rather than set the text to empty? (also requires preserving images, etc). Or perhaps the viewer could provide a method to preserve labels for an item that we could override?
(In reply to comment #22) > This fix is causing expanded nodes in the debug viewers to flash on each > refresh. The viewer is supressing setData callbacks on expanded nodes which > does not allow us to preserve lables. This was necessary to prevent blank children - upon refresh, the viewer needs to traverse all expanded nodes to let the content provider update the corresponding elements and child counts. Let's continue the discussion on bug 183463.
Verified on Windows XP using I20070503-0842.
