Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.
Bug 161805 - [Viewers] JFace TreeViewer collapses node when sibbling removed
Summary: [Viewers] JFace TreeViewer collapses node when sibbling removed
Status: VERIFIED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 3.2   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.3 M3   Edit
Assignee: Boris Bokowski CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2006-10-20 17:54 EDT by Christophe Cornu CLA
Modified: 2006-11-06 16:02 EST (History)
2 users (show)

See Also:


Attachments
Screenshot showing the tree before and after the remove. See the undesired collapsed node ater the removal (4.98 KB, image/png)
2006-10-20 17:55 EDT, Christophe Cornu CLA
no flags Details
The modified SampleView.java (6.18 KB, text/plain)
2006-10-20 17:56 EDT, Christophe Cornu CLA
no flags Details
patch (2.18 KB, patch)
2006-10-30 11:09 EST, Boris Bokowski CLA
no flags Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Christophe Cornu CLA 2006-10-20 17:54:52 EDT
3.2

1. Use the Eclipse Wizard to produce a plugin with a tree viewer view.
2. Paste the modified SampleView.java
- this looks like the left part of the attached screenshot
3. Click on the top node Parent1 and type -
4. TreeViewer.remove is called to remove Parent 1. TreeViewer.refresh is called
- this looks like the right part of the attached screenshot

Wrong: node Parent 4 is now collapsed.
Expectation Parent 4 is still expanded.

Is there any workaround? Am I misusing your API?

Note. The snippet is almost identical to the code generated by PDE for plugin / view / Tree Viewer. Here is the extra code in createPartControl:

		viewer.getTree().addKeyListener(new KeyListener() {
		
			public void keyReleased(KeyEvent e) {
				// TODO Auto-generated method stub
		
			}
		
			public void keyPressed(KeyEvent e) {
				if (e.character == '-') {
					TreeParent parent = (TreeParent) ((IStructuredSelection)viewer.getSelection()).toArray()[0];
					parent.getParent().removeChild(parent);
					viewer.refresh();
				}
				if (e.character == '+') {
					TreeParent parent = (TreeParent) ((IStructuredSelection)viewer.getSelection()).toArray()[0];
					parent.addChild(new TreeParent(new Date().toString()));
				}
					
			}
		
		});
Comment 1 Christophe Cornu CLA 2006-10-20 17:55:56 EDT
Created attachment 52434 [details]
Screenshot showing the tree before and after the remove. See the undesired collapsed node ater the removal
Comment 2 Christophe Cornu CLA 2006-10-20 17:56:58 EDT
Created attachment 52436 [details]
The modified SampleView.java
Comment 3 Christophe Cornu CLA 2006-10-20 18:06:32 EDT
Correction: 

4. Node "Parent 1" is removed from its parent node in the model. Then JFace TreeViewer.refresh is called
Comment 4 Boris Bokowski CLA 2006-10-25 15:53:59 EDT
Which build are you using?
Comment 5 Christophe Cornu CLA 2006-10-26 16:29:35 EDT
Old one. I am on 3.2
Build id: M20060629-1905

Mmh I'd be so happy if you tell me that does not occur anymore in the most recent IBuild. I can help you reproduce the problem if my instructions left you confused.
Comment 6 Boris Bokowski CLA 2006-10-30 11:09:03 EST
Created attachment 52935 [details]
patch
Comment 7 Boris Bokowski CLA 2006-10-30 11:16:33 EST
The reason for not preserving the expansion state in your example is that we reuse tree items on refresh, and we only preserve the expansion state for one level.

Fix released >20061030-0800.

The fix I released optimizes refresh for the case where elements are removed but not reordered. If elements are reordered, expansion state might still be lost.

Note that calling add/remove instead of refresh is the recommended practice if you want the tree viewer to do the minimal amount of work with the least amount of changes to the tree.
Comment 8 Boris Bokowski CLA 2006-11-01 10:42:09 EST
Verified using I20061031-0656.
Comment 9 Christophe Cornu CLA 2006-11-06 16:02:57 EST
Thanks Boris.