From a4d7b0e345c34859b5854c02e18602b72da4a37d Mon Sep 17 00:00:00 2001 From: Jeanette Winzenburg Date: Wed, 26 Jan 2022 16:17:53 +0100 Subject: [PATCH] 8187309: TreeCell must not change tree's data --- .../java/javafx/scene/control/TreeCell.java | 7 +- .../java/javafx/scene/control/TreeView.java | 7 ++ .../javafx/scene/control/TreeCellTest.java | 80 +++++++++++++++++++ .../javafx/scene/control/TreeViewTest.java | 3 +- 4 files changed, 91 insertions(+), 6 deletions(-) diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/TreeCell.java b/modules/javafx.controls/src/main/java/javafx/scene/control/TreeCell.java index b3911bd749f..e21fcedaecf 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/TreeCell.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/TreeCell.java @@ -414,12 +414,9 @@ public String getName() { newValue)); } + // FIXME: JDK-8187314 must respect actual committed value // update the item within this cell, so that it represents the new value - if (treeItem != null) { - treeItem.setValue(newValue); - updateTreeItem(treeItem); - updateItem(newValue, false); - } + updateItem(newValue, false); if (tree != null) { // reset the editing item in the TreetView diff --git a/modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java b/modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java index 4efd10e9415..a0f9cec45b7 100644 --- a/modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java +++ b/modules/javafx.controls/src/main/java/javafx/scene/control/TreeView.java @@ -337,6 +337,8 @@ public TreeView(TreeItem root) { MultipleSelectionModel> sm = new TreeViewBitSetSelectionModel(this); setSelectionModel(sm); setFocusModel(new TreeViewFocusModel(this)); + + setOnEditCommit(DEFAULT_EDIT_COMMIT_HANDLER); } @@ -845,6 +847,11 @@ public final ObjectProperty>> onEditCommitProperty() { return onEditCommit; } + private EventHandler> DEFAULT_EDIT_COMMIT_HANDLER = t -> { + TreeItem editedItem = t.getTreeItem(); + if (editedItem == null) return; + editedItem.setValue(t.getNewValue()); + }; // --- On Edit Cancel private ObjectProperty>> onEditCancel; diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeCellTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeCellTest.java index 2c2ff29aae0..bce8a459d7c 100644 --- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeCellTest.java +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeCellTest.java @@ -51,6 +51,7 @@ import javafx.scene.control.TreeItem; import javafx.scene.control.TreeView; import javafx.scene.control.TreeView.EditEvent; +import javafx.scene.control.cell.TextFieldTreeCell; import javafx.scene.control.skin.TreeCellSkin; import test.com.sun.javafx.scene.control.infrastructure.StageLoader; import test.com.sun.javafx.scene.control.infrastructure.VirtualFlowTestUtils; @@ -888,6 +889,85 @@ public void testCommitEditMustNotFireCancel() { assertEquals("commit must not have fired editCancel", 0, events.size()); } +//------------- JDK-8187309: fix editing mechanics to comply to specification + + @Test + public void testTreeHasDefaultCommitHandler() { + assertNotNull("treeView must have default commit handler", tree.getOnEditCommit()); + } + + @Test + public void testDefaultCommitUpdatesData() { + TreeItem editingItem = setupForEditing(cell); + tree.edit(editingItem); + String value = "edited"; + cell.commitEdit(value); + assertEquals("value committed", value, editingItem.getValue()); + } + + @Test + public void testDefaultCommitUpdatesCell() { + TreeCell cell = TextFieldTreeCell.forTreeView().call(tree); + TreeItem editingItem = setupForEditing(cell); + tree.edit(editingItem); + String value = "edited"; + cell.commitEdit(value); + assertEquals("cell text updated to committed value", value, cell.getText()); + } + + @Test + public void testDoNothingCommitHandlerDoesNotUpdateData() { + TreeItem editingItem = setupForEditing(cell); + String oldValue = editingItem.getValue(); + // do nothing handler + tree.setOnEditCommit(e -> {}); + tree.edit(editingItem); + String value = "edited"; + cell.commitEdit(value); + assertEquals("edited value must not be committed", oldValue, editingItem.getValue()); + } + + @Ignore("JDK-8187314") + @Test + public void testDoNothingCommitHandlerDoesNotUpdateCell() { + TreeCell cell = TextFieldTreeCell.forTreeView().call(tree); + TreeItem editingItem = setupForEditing(cell); + String oldValue = editingItem.getValue(); + // do nothing handler + tree.setOnEditCommit(e -> {}); + tree.edit(editingItem); + String value = "edited"; + cell.commitEdit(value); + assertEquals("cell text must not have changed", oldValue, cell.getText()); + } + + + /** + * Sets tree editable, configures the given cell for editing in tree at index 1 + * and returns the treeItem at that index. + */ + private TreeItem setupForEditing(TreeCell editingCell) { + tree.setEditable(true); + editingCell.updateTreeView(tree); + editingCell.updateIndex(1); + return editingCell.getTreeItem(); + } + + /** + * Test test setup. + */ + @Test + public void testSetupForEditing() { + TreeCell cell = new TreeCell<>(); + TreeItem cellTreeItem = setupForEditing(cell); + assertTrue("sanity: tree must be editable", tree.isEditable()); + assertEquals("sanity: returned treeItem", cellTreeItem, cell.getTreeItem()); + assertEquals(1, cell.getIndex()); + assertEquals("sanity: cell configured with tree's treeItem at index", + tree.getTreeItem(cell.getIndex()), cell.getTreeItem()); + assertNull("sanity: config doesn't change tree state", tree.getEditingItem()); + } + // When the tree view item's change and affects a cell that is editing, then what? // When the tree cell's index is changed while it is editing, then what? diff --git a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeViewTest.java b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeViewTest.java index 178b61ec634..9857a79e1ca 100644 --- a/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeViewTest.java +++ b/modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeViewTest.java @@ -1195,7 +1195,8 @@ public void updateItem(String item, boolean empty) { treeView.setOnEditStart(t -> { rt_29650_start_count++; }); - treeView.setOnEditCommit(t -> { + // Note: must add a commit handler to not replace the default (saving) handler + treeView.addEventHandler(TreeView.editCommitEvent(), t -> { rt_29650_commit_count++; }); treeView.setOnEditCancel(t -> {