Bug 37651 - [Viewers] StructuredViewer.update calls LabelProvider before updating expandedState
Summary: [Viewers] StructuredViewer.update calls LabelProvider before updating expande...
Status: RESOLVED WORKSFORME
Alias: None
Product: Platform
Classification: Eclipse Project
Component: UI (show other bugs)
Version: 2.1   Edit
Hardware: PC Windows 2000
: P3 normal (vote)
Target Milestone: 3.0 M9   Edit
Assignee: Nick Edgar CLA
QA Contact:
URL:
Whiteboard:
Keywords: api
Depends on:
Blocks:
 
Reported: 2003-05-15 09:43 EDT by George C. Hetrick CLA
Modified: 2004-05-07 13:13 EDT (History)
2 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description George C. Hetrick CLA 2003-05-15 09:43:16 EDT
In the program below, tree nodes are correctly reported as collapsed when they 
are first created, but if they are opened and closed, their state is reported 
incorrectly.

package bugs;

import org.eclipse.jface.viewers.ITreeContentProvider;
import org.eclipse.jface.viewers.ITreeViewerListener;
import org.eclipse.jface.viewers.LabelProvider;
import org.eclipse.jface.viewers.TreeExpansionEvent;
import org.eclipse.jface.viewers.TreeViewer;
import org.eclipse.jface.viewers.Viewer;
import org.eclipse.swt.SWT;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.widgets.Display;
import org.eclipse.swt.widgets.Shell;

public class BugReport {
	private static ITreeContentProvider contentProvider =
		new MyContentProvider();
	private static MyLabelProvider labelProvider = new MyLabelProvider();
	private static TreeViewer treeViewer;

	private static String root = "root";
	private static String one = "one";
	private static String two = "two";
	private static String three = "three";

	static class MyLabelProvider
		extends LabelProvider
		implements ITreeViewerListener {
		public String getText(Object element) {
			return ((String) element)
				+ ((treeViewer.getExpandedState(element))
					? ":expanded"
					: ":closed");
		}
		public void treeCollapsed(TreeExpansionEvent event) {
			event.getTreeViewer().update(event.getElement(), null);
		}

		public void treeExpanded(TreeExpansionEvent event) {
			event.getTreeViewer().update(event.getElement(), null);
		}
	};

	static class MyContentProvider implements ITreeContentProvider {
		public Object[] getChildren(Object parentElement) {
			if (parentElement.equals(root))
				return new Object[] { one };
			if (parentElement.equals(one))
				return new Object[] { two };
			if (parentElement.equals(two))
				return new Object[] { three };
			return new Object[0];
		}

		public Object getParent(Object element) {
			if (element.equals(one))
				return root;
			if (element.equals(two))
				return one;
			if (element.equals(three))
				return two;
			return null;
		}

		public boolean hasChildren(Object element) {
			return element != three;
		}

		public Object[] getElements(Object inputElement) {
			return getChildren(inputElement);
		}

		public void dispose() {}

		public void inputChanged(
			Viewer viewer,
			Object oldInput,
			Object newInput) {
			}
	};

	public static void main(String[] args) {
		final Display display = Display.getDefault();
		Shell shell = new Shell(display, SWT.TITLE | SWT.ICON | 
SWT.CLOSE);
		shell.setLayout(new FillLayout());
		treeViewer = new TreeViewer(shell);
		treeViewer.addTreeListener(labelProvider);
		treeViewer.setContentProvider(contentProvider);
		treeViewer.setLabelProvider(labelProvider);
		treeViewer.setInput(root);
		shell.open();

		// start the event loop. We stop when the user has done
		// something to dispose our window.
		//
		while (!shell.isDisposed()) {
			if (!display.readAndDispatch())
				display.sleep();
		}
		display.dispose();
	}

}
Comment 1 Steve Northover CLA 2003-05-16 10:20:15 EDT
Looks like JFace to me.  Please move back if it turns out to be SWT.  Thanks.
Comment 2 Nick Edgar CLA 2003-05-20 10:18:43 EDT
It is not StructuredViewer.update's responsibility to update the expanded state.

Te problem is that during the Tree expand or collapse callback from SWT, the 
expanded/collapsed state of the item has not yet changed.  A better name for 
the event would be treeExpanding/treeCollapsing.  The JFace event name is 
consistent with SWT's TreeListener.  Note that the underlying SWT event name 
constants are SWT.Expand/SWT.Collapse.

We have received many PRs and newsgroup questions about this behaviour.  We 
should do something to address it.  At a minimum, the specs for the events at 
both the SWT and JFace level should be clarified.  A name change would be 
better.
Comment 3 George C. Hetrick CLA 2003-05-20 14:47:01 EDT
Is there something different that I, as a coder, should be doing? Nick's 
comment seems to suggest a documentation change, rather than a behavior change.
Comment 4 Nick Edgar CLA 2003-05-20 15:10:44 EDT
Correct, this is a spec. problem, not an implementation bug.
One workaround is to queue the update using Display.asyncExec so that it gets 
run after the treeExpanded/treeCollapsed callback returns.

For example:
	public void treeCollapsed(final TreeExpansionEvent event) {
		final AbstractTreeViewer viewer = event.getTreeViewer();
		viewer.getControl().getDisplay().asyncExec(
			new Runnable() {
				public void run() {
					viewer.update(event.getElement(), null);
				}
			}
		);
	}
Comment 5 Nick Edgar CLA 2004-05-07 11:35:33 EDT
Moving to SWT for comment.
Steve, is it the intent that a TreeItem.getExpanded() call during a Collapse 
event should still return true (and vice versa for Expand)?
Are there platform differences here?

Can you comment on the suggested workaround above?
Comment 6 Steve Northover CLA 2004-05-07 12:45:46 EDT
This is the intent.  I have verified that the behavior is the same on every 
platform.
Comment 7 Nick Edgar CLA 2004-05-07 13:13:08 EDT
ITreeViewerListener's method names could be improved, but that would be a
breaking API change.

Will add a clarification to the Javadoc.