TreeViewer.setSelection() does not select the correct item after the path given

True Soft picture True Soft · Jun 9, 2011 · Viewed 8k times · Source

If there are more tree items based on the same object in the tree viewer, making a TreePath and passing it to TreeViewer.setSelection() does not select the item correctly when the current selection is equal to the one I want to navigate.

Example:
There's a tree with 2 items that show the same object (BigDecimal.ONE in this case). They have different paths (different parents): tree

I want that when I am on one BigDecimal.ONE item, to click on the link and navigate to the other BigDecimal.ONE. On the selection listener of the link I make a TreeSelection with a correct TreePath. Then I call setSelection. But the navigation does not work. However, if the root item is initially collapsed, I notice that it expands it, but does not navigate to the correct item.

The code is this:

import java.math.BigDecimal;
import java.util.ArrayList;
import java.util.List;

import org.eclipse.jface.viewers.*;
import org.eclipse.jface.window.ApplicationWindow;
import org.eclipse.jface.window.Window;
import org.eclipse.swt.SWT;
import org.eclipse.swt.events.SelectionAdapter;
import org.eclipse.swt.events.SelectionEvent;
import org.eclipse.swt.graphics.Image;
import org.eclipse.swt.layout.FillLayout;
import org.eclipse.swt.widgets.*;

public class TreeViewerExample {

    static class Model {
        String   name;
        Number[] children;

        public Model(String name, Number... numbers) {
            this.name = name;
            this.children = numbers;
        }

        public String toString() {
            return name;
        }
    }

    static class ModelTreeProvider extends LabelProvider implements ITableLabelProvider, ITreeContentProvider {

        public Object[] getChildren(Object parentElement) {
            if (parentElement instanceof Model) {
                return ((Model) parentElement).children;
            } else {
                return new Object[0];
            }
        }
        public Object getParent(Object element) {
            System.err.println("requesting the parent for " + element);
            return null;
        }
        public boolean hasChildren(Object element)
        {
            return getChildren(element) == null ? false : getChildren(element).length > 0;
        }
        public Object[] getElements(Object inputElement)
        {
            return (inputElement instanceof List)? ((List) inputElement).toArray():new Object[0];
        }
        public void dispose()
        {
        }
        public void inputChanged(Viewer arg0, Object arg1, Object arg2)
        {
        }
        public String getColumnText(Object element, int columnIndex)
        {
            return element.toString();
        }
        public Image getColumnImage(Object element, int columnIndex)
        {
            return null;
        }
    }

    public static void main(String[] args) {
        final List<Model> models = new ArrayList<Model>();
        models.add(new Model("Zero and one", BigDecimal.ZERO, BigDecimal.ONE));
        models.add(new Model("One and ten", BigDecimal.ONE, BigDecimal.TEN));

        Window app = new ApplicationWindow(null) {
            private TreeViewer treeViewer;
            private Link       link;
            protected Control createContents(Composite parent) {
                Composite composite = new Composite(parent, SWT.NONE);
                composite.setLayout(new FillLayout());
                treeViewer = new TreeViewer(composite);
                ModelTreeProvider provider = new ModelTreeProvider();
                treeViewer.setContentProvider(provider);
                treeViewer.setLabelProvider(provider);
                treeViewer.setInput(models);
                treeViewer.getTree().addSelectionListener(new SelectionAdapter() {
                    @Override
                    public void widgetSelected(SelectionEvent e) {
                        if (((TreeItem) e.item).getText().equals("1")) {
                            link.setText("This is from "+((TreeItem) e.item).getParentItem().getText()
                                    + "\r\n<a href=\"go\">Go to the other " + ((TreeItem) e.item).getText() + "</a>");
                        } else {
                            link.setText(" - ");
                        }
                        link.setData(e.item);
                    }
                });
                link = new Link(composite, SWT.NONE);
                link.setText(" - ");
                link.addSelectionListener(new SelectionAdapter() {
                    public void widgetSelected(SelectionEvent e) {
                        List<Object> path = new ArrayList<Object>();
                        if (treeViewer.getTree().indexOf(treeViewer.getTree().getSelection()[0].getParentItem()) == 0)
                        {// if the first is selected, go to the second
                            path.add(treeViewer.getTree().getItem(1).getData());
                        } else {
                            path.add(treeViewer.getTree().getItem(0).getData());
                        }
                        path.add(BigDecimal.ONE);
                        treeViewer.setSelection(new TreeSelection(new TreePath(path.toArray())), true);
                    }
                });
                return composite;
            }
        };
        app.setBlockOnOpen(true);
        app.open();
    }
}

My question is if this a jface bug or am I not doing this the right way?


EDIT:

It looks that there is already a posted bug on eclipse.org: https://bugs.eclipse.org/bugs/show_bug.cgi?id=332736

Answer

Danail Nachev picture Danail Nachev · Jun 10, 2011

It looks like this is a bug in JFace. After all, you are passing TreePath so it should figure it out which exactly instance of your object you want.

The problem is the org.eclipse.jface.viewers.AbstractTreeViewer.isSameSelection(List, Item[]) method. It incorrectly determine, based on the element found in the items (the BigIntegers) that the selection is the same. It should have checked the whole tree path to make sure that the selection will be indeed the same. Fortunately, you can fix this in your code by overriding the isSameSelection(List, Item[]) method and correctly check against the tree paths instead of the elements themselves:

       treeViewer = new TreeViewer(composite) {
            protected boolean isSameSelection(List items, Item[] current) {
                // If they are not the same size then they are not equivalent
                int n = items.size();
                if (n != current.length) {
                    return false;
                }
                Set itemSet = new HashSet(n * 2 + 1);
                for (Iterator i = items.iterator(); i.hasNext();) {
                    Item item = (Item) i.next();
                    itemSet.add(getTreePathFromItem(item));
                }
                // Go through the items of the current collection
                // If there is a mismatch return false
                for (int i = 0; i < current.length; i++) {
                    if (current[i].getData() == null
                            || !itemSet.contains(getTreePathFromItem(current[i]))) {
                        return false;
                    }
                }
                return true;
            }
       };

This however will fix this particular problem, but there is no guarantee that there are other problems waiting to be found. Personally, I always try to stay away of having the same elements in different places of the tree, just in case.