Summary: | [Viewers] JFace tree viewer collapses tree items on refresh | ||
---|---|---|---|
Product: | [Eclipse Project] Platform | Reporter: | Alex Nan <apnan> |
Component: | UI | Assignee: | Boris Bokowski <bokowski> |
Status: | RESOLVED DUPLICATE | QA Contact: | |
Severity: | normal | ||
Priority: | P3 | CC: | popescu |
Version: | 2.1 | ||
Target Milestone: | --- | ||
Hardware: | PC | ||
OS: | Windows 2000 | ||
Whiteboard: |
Description
Alex Nan
2003-03-24 12:29:41 EST
For 2.1, a bug was fixed whereby the expanded/collapsed state was being carried forward on an item for which the corresponding model element was changed. For example, when deleting an expanded element, the element that took its place would then be expanded. When you issue the refresh, are the elements that are getting collapsed equal to the old elements? The viewer preserves the expansion state for equal elements, but not for unequal elements, even if they end up in the same position. The scenario is the following: a filter is applied that is supposed to filter
out first level nodes which have a property in a certain range of values, to be
more specific lets say severity, which I denote by s, that could be in 0<=s<30
low severity range, 30<=s<50 medium or 50<=s<70 high. The tree has the
following structure: it has N first level nodes, which all have a severity
property set and that could have children in a depth of 4, so the tree can
actually have 5 levels of nodes. On refresh the getElements() is returning only
the first level objects (and their children) which comply to the filter
criteria (low, medium ,high, all, low or medium etc.) but in fact the nodes at
levels > 1 are the same, nothing has changed in their structure, i.e getElements
() on objects in levels >1 is returning the same objects. Every node at levels
>2 will be for some reason collapsed and as a consequence any selection on
those nodes will be lost. Hopefully my explanations will help, the same
scenario used to work in 2.0.2.
When getChildren/getElements returns the elements, are they equal to the previous ones? That is, do your model elements implement equals(Object) and is the result true when comparing the element before and after the refresh? No unfortunately, now I realize that all the elements on level 2 are dynamically created every time a refresh is called by introspecting the objects on level 1 which in fact are not changing. This explains now the strange behaviour. What to do? Actually the level 2 nodes contain the same information before and after the refresh but I create on every refresh a new wrapper class, which is probably not a good ideea. I wrote some code that caches a level 2 node once it is created, I'm not sure what performance impact this approach will have, anyway the nodes are not collapsed anymore! Thanks. I recommend implementing equals(Object) on your model objects that compares based on some identifying criteria of the objects (if they're wrappers, you could compare the wrapped elements). Don't compare attributes that may change during the lifetime of the object. This may be cheaper and easier to manage than remembering the wrappers in hash tables. I'll keep this PR open for consideration post 2.1 since I'm not totally satisfied with the new viewer behaviour. Although the original problem with expanded state getting passed on to the wrong elements was fixed, there are several other cases where this wasn't such a bad thing. For example, one bad side effect of the new behaviour is that if you rename an element, and the name is part of the equality criteria, then the viewer has no idea that the user still thinks of this as the same element. So if it was expanded before, after a refresh it is collapsed, even though it's still in the same position. It might be better to process deleted elements specially (delete the corresponding SWT item rather than reusing it so that the expanded state is not passed on to the next element), and then just let the expanded state for the remaining items to be passed on (as in 2.0). The problem with this approach is that it does not handle reordering of elements, e.g. when changing the sort order. OK, I implement equals() for the wrapper but what do I return on getElements (Object element) for a first level node, because in the current code I'm introspecting element and I'm returning a wrapper object for every EMF feature of element that satisfies a filter criteria. I'm doing something like this elementsList.add(new FeatureNode(Ebject element, EStructuralFeature feature)). If I'm caching FeatureNode then on the next refresh I don't have to return a new feature wrapper object and I will avoid the collapsing problem on an expanded FeatureNode. How is the equals(Object) helping me in my case? My problem is to avoid the collapsing of nodes, so I have to provide the same object to the viewer. I assume you are using equals to compare the old and new objects, or perhaps I did't understand your suggestion. Thank you. If FeatureNode does not reimplement equals(Object) then it gets the default behaviour which compares object identity. If you're caching the items, then this works fine. But there is overhead to maintaining the cache (e.g. do you let the table grow indefinitely, or do you need to track when items can be removed). Another way of doing it is to implement equals(Object) on FeatureNode so that the following returns true: FeatureNode a = new FeatureNode(element, feature); FeatureNode b = new FeatureNode(element, feature); a.equals(b) So even if you're recreating the FeatureNode wrapper each time, the elements are still equal as long as the underlying element and feature are equal. Note that you'd also have to implement hashCode() accordingly. I followed your tip but unfortunately I still have the problem, this time it happens only in the following scenario: - suppose I have node A, B and C which all have subnodes and I expand A and C to their second level children and filter out B, after refreshing the view the subnodes of A are still expanded but not those of C although the subnodes of C are the same and equals(Object elem)) returns true for them. The only change is that the second time getElements() is returning only A and C, so C has changed his place and moved upwards. I've also noticed the fact that the viewer is slow on refresh operations, I profiled a refresh for a tree that had 2048 first level nodes. It seems that AbstractTreeViewer.updateChildren() takes a lot of time compared to other methods and also AbstractTreeViewer.isExpandable() is called twice for every node, which makes the hasChildren() method in the ContentProvider to be called the same nr. of times. As a consequence if hasChildren() is doing a bit of work this could slow the viewer even more. For the case in comment #8, it's important that equals returns true for C, when comparing the old and new object representing C. If it already does this, then I'm not sure what's going on. You'd have to set a breakpoint and trace through updateChildren, which tries to preserve the expansion state. In a profile, updateChildren will take most of the time, since it does most of the work. There are some inefficiencies due to the way the code is structured, so hasChildren will get called twice for each element. Note that you can implement hasChildren in such a way that it returns true if that kind of element ever has children; you don't need to check whether it actually has children. For example, it could always return true for an object representing a folder. Reassigning bugs in component areas that are changing ownership. *** This bug has been marked as a duplicate of bug 113675 *** |