diff --git a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeCellSkin.java b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeCellSkin.java --- a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeCellSkin.java +++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeCellSkin.java @@ -215,7 +215,7 @@ Node disclosureNode = getSkinnable().getDisclosureNode(); - int level = TreeView.getNodeLevel(treeItem); + int level = tree.getTreeItemLevel(treeItem); if (! tree.isShowRoot()) level--; double leftMargin = getIndent() * level; @@ -303,7 +303,7 @@ pw = labelWidth; // determine the amount of indentation - int level = TreeView.getNodeLevel(treeItem); + int level = tree.getTreeItemLevel(treeItem); if (! tree.isShowRoot()) level--; pw += getIndent() * level; diff --git a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableCellSkin.java b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableCellSkin.java --- a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableCellSkin.java +++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableCellSkin.java @@ -91,7 +91,7 @@ TreeItem treeItem = treeTableRow.getTreeItem(); if (treeItem == null) return leftPadding; - int nodeLevel = TreeTableView.getNodeLevel(treeItem); + int nodeLevel = treeTable.getTreeItemLevel(treeItem); if (! treeTable.isShowRoot()) nodeLevel--; double indentPerLevel = 10; diff --git a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableRowSkin.java b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableRowSkin.java --- a/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableRowSkin.java +++ b/modules/controls/src/main/java/com/sun/javafx/scene/control/skin/TreeTableRowSkin.java @@ -231,7 +231,7 @@ } @Override protected int getIndentationLevel(TreeTableRow control) { - return TreeTableView.getNodeLevel(control.getTreeItem()); + return control.getTreeTableView().getTreeItemLevel(control.getTreeItem()); } @Override protected double getIndentationPerLevel() { diff --git a/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java b/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java --- a/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java +++ b/modules/controls/src/main/java/javafx/scene/control/TreeTableView.java @@ -463,18 +463,29 @@ } private static final EventType EDIT_COMMIT_EVENT = new EventType(editAnyEvent(), "EDIT_COMMIT"); - + /** * Returns the number of levels of 'indentation' of the given TreeItem, - * based on how many times getParent() can be recursively called. If the - * given TreeItem is the root node, or if the TreeItem does not have any - * parent set, the returned value will be zero. For each time getParent() is - * recursively called, the returned value is incremented by one. - * + * based on how many times {@link javafx.scene.control.TreeItem#getParent()} + * can be recursively called. If the TreeItem does not have any parent set, + * the returned value will be zero. For each time getParent() is recursively + * called, the returned value is incremented by one. + * + *

Important note: This method is deprecated as it does + * not consider the root node. This means that this method will iterate + * past the root node of the TreeTableView control, if the root node has a parent. + * If this is important, call {@link TreeTableView#getTreeItemLevel(TreeItem)} + * instead. + * * @param node The TreeItem for which the level is needed. * @return An integer representing the number of parents above the given node, * or -1 if the given TreeItem is null. + * @deprecated This method does not correctly calculate the distance from the + * given TreeItem to the root of the TreeTableView. As of JavaFX 8.0_20, + * the proper way to do this is via + * {@link TreeTableView#getTreeItemLevel(TreeItem)} */ + @Deprecated public static int getNodeLevel(TreeItem node) { return TreeView.getNodeLevel(node); } @@ -1496,6 +1507,39 @@ treeItemCacheMap.put(_row, new SoftReference<>(treeItem)); return treeItem; } + + /** + * Returns the number of levels of 'indentation' of the given TreeItem, + * based on how many times getParent() can be recursively called. If the + * given TreeItem is the root node of this TreeTableView, or if the TreeItem + * does not have any parent set, the returned value will be zero. For each + * time getParent() is recursively called, the returned value is incremented + * by one. + * + * @param node The TreeItem for which the level is needed. + * @return An integer representing the number of parents above the given node, + * or -1 if the given TreeItem is null. + */ + public int getTreeItemLevel(TreeItem node) { + final TreeItem root = getRoot(); + + if (node == null) return -1; + if (node == root) return 0; + + int level = 0; + TreeItem parent = node.getParent(); + while (parent != null) { + level++; + + if (parent == root) { + break; + } + + parent = parent.getParent(); + } + + return level; + } /** * The TreeTableColumns that are part of this TableView. As the user reorders diff --git a/modules/controls/src/main/java/javafx/scene/control/TreeView.java b/modules/controls/src/main/java/javafx/scene/control/TreeView.java --- a/modules/controls/src/main/java/javafx/scene/control/TreeView.java +++ b/modules/controls/src/main/java/javafx/scene/control/TreeView.java @@ -254,15 +254,26 @@ /** * Returns the number of levels of 'indentation' of the given TreeItem, - * based on how many times getParent() can be recursively called. If the - * given TreeItem is the root node, or if the TreeItem does not have any - * parent set, the returned value will be zero. For each time getParent() is - * recursively called, the returned value is incremented by one. + * based on how many times {@link javafx.scene.control.TreeItem#getParent()} + * can be recursively called. If the TreeItem does not have any parent set, + * the returned value will be zero. For each time getParent() is recursively + * called, the returned value is incremented by one. + * + *

Important note: This method is deprecated as it does + * not consider the root node. This means that this method will iterate + * past the root node of the TreeView control, if the root node has a parent. + * If this is important, call {@link TreeView#getTreeItemLevel(TreeItem)} + * instead. * * @param node The TreeItem for which the level is needed. * @return An integer representing the number of parents above the given node, - * or -1 if the given TreeItem is null. + * or -1 if the given TreeItem is null. + * @deprecated This method does not correctly calculate the distance from the + * given TreeItem to the root of the TreeView. As of JavaFX 8.0_20, + * the proper way to do this is via + * {@link TreeView#getTreeItemLevel(TreeItem)} */ + @Deprecated public static int getNodeLevel(TreeItem node) { if (node == null) return -1; @@ -276,7 +287,7 @@ return level; } - + /*************************************************************************** * * * Constructors * @@ -951,6 +962,38 @@ return treeItem; } + /** + * Returns the number of levels of 'indentation' of the given TreeItem, + * based on how many times getParent() can be recursively called. If the + * given TreeItem is the root node of this TreeView, or if the TreeItem does + * not have any parent set, the returned value will be zero. For each time + * getParent() is recursively called, the returned value is incremented by one. + * + * @param node The TreeItem for which the level is needed. + * @return An integer representing the number of parents above the given node, + * or -1 if the given TreeItem is null. + */ + public int getTreeItemLevel(TreeItem node) { + final TreeItem root = getRoot(); + + if (node == null) return -1; + if (node == root) return 0; + + int level = 0; + TreeItem parent = node.getParent(); + while (parent != null) { + level++; + + if (parent == root) { + break; + } + + parent = parent.getParent(); + } + + return level; + } + /** {@inheritDoc} */ @Override protected Skin createDefaultSkin() { return new TreeViewSkin(this); diff --git a/modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java b/modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java --- a/modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java +++ b/modules/controls/src/test/java/javafx/scene/control/TreeTableViewTest.java @@ -31,6 +31,7 @@ import static org.junit.Assert.*; import static org.junit.Assert.assertEquals; +import java.util.ArrayList; import java.util.Comparator; import java.util.List; import java.util.Random; @@ -3111,4 +3112,93 @@ group2.setExpanded(false); Toolkit.getToolkit().firePulse(); } + + @Test public void test_rt23245_itemIsInTree() { + final TreeTableView view = new TreeTableView(); + final List> items = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + final TreeItem item = new TreeItem("Item" + i); + item.setExpanded(true); + items.add(item); + } + + // link the items up so that the next item is the child of the current item + for (int i = 0; i < 9; i++) { + items.get(i).getChildren().add(items.get(i + 1)); + } + + view.setRoot(items.get(0)); + + for (int i = 0; i < 10; i++) { + // we expect the level of the tree item at the ith position to be + // 0, as every iteration we are setting the ith item as the root. + assertEquals(0, view.getTreeItemLevel(items.get(i))); + + // whilst we are testing, we should also ensure that the ith item + // is indeed the root item, and that the ith item is indeed the item + // at the 0th position + assertEquals(items.get(i), view.getRoot()); + assertEquals(items.get(i), view.getTreeItem(0)); + + // shuffle the next item into the root position (keeping its parent + // chain intact - which is what exposes this issue in the first place). + if (i < 9) { + view.setRoot(items.get(i + 1)); + } + } + } + + @Test public void test_rt23245_itemIsNotInTree_noRootNode() { + final TreeView view = new TreeView(); + final List> items = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + final TreeItem item = new TreeItem("Item" + i); + item.setExpanded(true); + items.add(item); + } + + // link the items up so that the next item is the child of the current item + for (int i = 0; i < 9; i++) { + items.get(i).getChildren().add(items.get(i + 1)); + } + + for (int i = 0; i < 10; i++) { + // because we have no root (and we are not changing the root like + // the previous test), we expect the tree item level of the item + // in the ith position to be i. + assertEquals(i, view.getTreeItemLevel(items.get(i))); + + // all items requested from the TreeView should be null, as the + // TreeView does not have a root item + assertNull(view.getTreeItem(i)); + } + } + + @Test public void test_rt23245_itemIsNotInTree_withUnrelatedRootNode() { + final TreeView view = new TreeView(); + final List> items = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + final TreeItem item = new TreeItem("Item" + i); + item.setExpanded(true); + items.add(item); + } + + // link the items up so that the next item is the child of the current item + for (int i = 0; i < 9; i++) { + items.get(i).getChildren().add(items.get(i + 1)); + } + + view.setRoot(new TreeItem("Unrelated root node")); + + for (int i = 0; i < 10; i++) { + // because we have no root (and we are not changing the root like + // the previous test), we expect the tree item level of the item + // in the ith position to be i. + assertEquals(i, view.getTreeItemLevel(items.get(i))); + + // all items requested from the TreeView should be null except for + // the root node + assertNull(view.getTreeItem(i + 1)); + } + } } diff --git a/modules/controls/src/test/java/javafx/scene/control/TreeViewTest.java b/modules/controls/src/test/java/javafx/scene/control/TreeViewTest.java --- a/modules/controls/src/test/java/javafx/scene/control/TreeViewTest.java +++ b/modules/controls/src/test/java/javafx/scene/control/TreeViewTest.java @@ -36,10 +36,7 @@ import com.sun.javafx.scene.control.test.RT_22463_Person; import com.sun.javafx.tk.Toolkit; -import java.util.Arrays; -import java.util.Collections; -import java.util.Comparator; -import java.util.List; +import java.util.*; import javafx.beans.InvalidationListener; import javafx.beans.Observable; @@ -1511,4 +1508,93 @@ group2.setExpanded(false); Toolkit.getToolkit().firePulse(); } + + @Test public void test_rt23245_itemIsInTree() { + final TreeView view = new TreeView(); + final List> items = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + final TreeItem item = new TreeItem("Item" + i); + item.setExpanded(true); + items.add(item); + } + + // link the items up so that the next item is the child of the current item + for (int i = 0; i < 9; i++) { + items.get(i).getChildren().add(items.get(i + 1)); + } + + view.setRoot(items.get(0)); + + for (int i = 0; i < 10; i++) { + // we expect the level of the tree item at the ith position to be + // 0, as every iteration we are setting the ith item as the root. + assertEquals(0, view.getTreeItemLevel(items.get(i))); + + // whilst we are testing, we should also ensure that the ith item + // is indeed the root item, and that the ith item is indeed the item + // at the 0th position + assertEquals(items.get(i), view.getRoot()); + assertEquals(items.get(i), view.getTreeItem(0)); + + // shuffle the next item into the root position (keeping its parent + // chain intact - which is what exposes this issue in the first place). + if (i < 9) { + view.setRoot(items.get(i + 1)); + } + } + } + + @Test public void test_rt23245_itemIsNotInTree_noRootNode() { + final TreeView view = new TreeView(); + final List> items = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + final TreeItem item = new TreeItem("Item" + i); + item.setExpanded(true); + items.add(item); + } + + // link the items up so that the next item is the child of the current item + for (int i = 0; i < 9; i++) { + items.get(i).getChildren().add(items.get(i + 1)); + } + + for (int i = 0; i < 10; i++) { + // because we have no root (and we are not changing the root like + // the previous test), we expect the tree item level of the item + // in the ith position to be i. + assertEquals(i, view.getTreeItemLevel(items.get(i))); + + // all items requested from the TreeView should be null, as the + // TreeView does not have a root item + assertNull(view.getTreeItem(i)); + } + } + + @Test public void test_rt23245_itemIsNotInTree_withUnrelatedRootNode() { + final TreeView view = new TreeView(); + final List> items = new ArrayList<>(); + for (int i = 0; i < 10; i++) { + final TreeItem item = new TreeItem("Item" + i); + item.setExpanded(true); + items.add(item); + } + + // link the items up so that the next item is the child of the current item + for (int i = 0; i < 9; i++) { + items.get(i).getChildren().add(items.get(i + 1)); + } + + view.setRoot(new TreeItem("Unrelated root node")); + + for (int i = 0; i < 10; i++) { + // because we have no root (and we are not changing the root like + // the previous test), we expect the tree item level of the item + // in the ith position to be i. + assertEquals(i, view.getTreeItemLevel(items.get(i))); + + // all items requested from the TreeView should be null except for + // the root node + assertNull(view.getTreeItem(i + 1)); + } + } }