Community
Participate
Working Groups
1) In the package explorer expand a compilation unit 2) double click a member to open the compilation unit -> the package explorer will quickly collapse the and expand the compilation unit. Similarly, when closing an editor then the corresponding CU is collapsed in the package explorer.
The content provider only calls viewer.refresh(<compilation unit>, false) exactly once if a member is opened or the editor is closed. Did something change in the JFace tree viewer. Moving to Platform UI for investigation.
Note: if I go back to AbstractTreeViewer rev. 1.2 it works.
Another scenario that is like caused by the same bug: JUnit setup 1) drill down to junit.framework.TestCase.java 2) expande the compilation unit 3) double click TestCase.java to open it in the editor 4) close the TestCase.java editor ->TestCase.java collapses to the top-level type. What is in commin in both scenarios is that the members are replaced by "working copy members" on open and with "original members" when closing the editor. I suspect that the tree viewer's behaviour was changed to collapse the nodes in this scenario.
Erich's suspicion is correct. This is a regression due to the fix for bug 3840. It now collapses the item for the type (not the CU) because the old type is not equal to the new type since one is a working copy and the other is not. This is actually correct behaviour. As far the tree viewer is concerned, the old type is gone, and the new type is new, since the two types are not equal. So it does not preserve the expansion state, even though the same SWT TreeItem ends up getting reused. It was previously relying on the incorrect behaviour prior to the fix for bug 3840. That explains the collapse. The expand occurs because ProblemTreeViewer.handleInvalidSelection (which is called because the selection has changed due to the selected method disappearing) converts the old non-working-set-based selection to the new working-set-based selection (or vice versa), and the tree expands the new type to show the selected method. I see 4 possible alternative solutions. 1. Unwind the fix for bug 3840. This is not a great answer, since this was a pretty annoying bug which was encountered fairly frequently. 2. Turn redraw off and on around the refresh. This is also not too tasty for three reasons: - you'd still see flicker - expansion state would still get lost for other elements besides the containing type which were expanded, but not along the path to the selected method (i.e. member types and secondary types) - the viewer still has to do extra work: when it collapsed the parent type, it deletes all its children, then has to repopulate them to show the new selection; this may take time, and may also cause a visible change (e.g. change to scroll position if the old methods were at the bottom of the tree) 3. Add support for pluggable element comparison (equals and hashCode) to the viewers (finally). So the Package Explorer could configure its viewer so that it ignores whether elements are working copies when comparing them (they're equal if they have the same underlying element). Note that viewer refresh already correctly handles updating its references to be the new elements, even if they are equal, so if we did this, after the refresh the viewer would be properly referring to the working set elements after the open (or vice versa). This would also eliminate the need to do the mapping in handleInvalidSelection. I'm not sure whether this would have other consequences for the Package Explorer though. 4. Package Explorer's viewer overrides TreeViewer.updateChildren to do this properly, either by removing the code that tracks the expanded set, or by changing how elements are compared. You'd probably want to do the latter so that you still get the fix for bug 3840 for cases not involving working copies, so this really boils down to a hacked special case of 3. But it would at least allow you to play around with a solution to see if 3 would really meet your needs. Thoughts? Do you see any other options?
Regarding the options: 1) agreed it isn't tasty. 2) not very tasty since drawing speed can differ between platforms I suspect it would be really bad on Motif 3) would be a clean solution 4) a workaround for 3). However, updateChildren is not code that we can afford to copy. New 5) JavaCore makes it transparent to the client whether there is a working copy or not. This was investigated but had to be rejected, since changing the semantics of JavaElements.equals() is a too drastic change. From the above the cleanest solution is 3) (see also bug 3496 for a discussion on this topic). It would fix a general issue. If 3) is not doable at this point in time than 1) would be a possible fall back. The old behaviour was less annoying than the new one.
I've backed out of the fix for 3840 for now (in tomorrow's integration build). Will look at adding equals/hashCode delegation for M5, in which case I'll put the fix back for 3840.
Added IElementComparer to org.eclipse.jface.viewers. This is plugged in to a viewer using StructuredViewer.setComparer (IElementComparer). This has been released for I20030128, along with re-releasing the fix for bug 3840 (so the free collapse/expand will be back in in I20030128 until the package explorer is changed to use this support). The old path, where comparer is not set, has been well tested using the existing JFace viewer test suite. The new path has not yet been extensively tested, but it should work well enough to try changing the package explorer to use it.
Leaving open for now to update the test suites.
Filed bug 32284 for the tests. Closing this one since the original problem has been solved.
