changeset 3834:374145c4e41e

Unit test and partial fix for RT-29849: [TreeTableView] on edit start is fired many times
author jgiles
date Mon, 03 Jun 2013 17:18:14 +1200
parents 7a327e81151a
children b7cc945d9169
files javafx-ui-controls/src/javafx/scene/control/TableCell.java javafx-ui-controls/src/javafx/scene/control/TreeCell.java javafx-ui-controls/src/javafx/scene/control/TreeTableCell.java javafx-ui-controls/test/com/sun/javafx/scene/control/infrastructure/VirtualFlowTestUtils.java javafx-ui-controls/test/javafx/scene/control/CellTest.java javafx-ui-controls/test/javafx/scene/control/ListViewKeyInputTest.java javafx-ui-controls/test/javafx/scene/control/TableViewKeyInputTest.java javafx-ui-controls/test/javafx/scene/control/TreeTableViewKeyInputTest.java javafx-ui-controls/test/javafx/scene/control/TreeViewKeyInputTest.java
diffstat 9 files changed, 238 insertions(+), 12 deletions(-) [+]
line wrap: on
line diff
--- a/javafx-ui-controls/src/javafx/scene/control/TableCell.java	Fri May 31 12:45:31 2013 +1200
+++ b/javafx-ui-controls/src/javafx/scene/control/TableCell.java	Mon Jun 03 17:18:14 2013 +1200
@@ -81,6 +81,16 @@
 
     /***************************************************************************
      *                                                                         *
+     * Private fields                                                          *
+     *                                                                         *
+     **************************************************************************/
+
+    // package for testing
+    boolean lockItemOnEdit = false;
+
+
+    /***************************************************************************
+     *                                                                         *
      * Callbacks and Events                                                    *
      *                                                                         *
      **************************************************************************/
@@ -267,18 +277,17 @@
         final TableColumn column = getTableColumn();
         if (! isEditable() ||
                 (table != null && ! table.isEditable()) ||
-                (column != null && ! getTableColumn().isEditable()))
-        {
-//            if (Logging.getControlsLogger().isLoggable(PlatformLogger.WARNING)) {
-//                Logging.getControlsLogger().warning(
-//                    "Can not call TableCell.startEdit() on this TableCell, as it "
-//                        + "is not allowed to enter its editing state (TableCell: "
-//                        + this + ", TableView: " + table + ").");
-//            }
-            
+                (column != null && ! getTableColumn().isEditable())) {
             return;
         }
-        
+
+        // We check the boolean lockItemOnEdit field here, as whilst we want to
+        // updateItem normally, when it comes to unit tests we can't have the
+        // item change in all circumstances.
+        if (! lockItemOnEdit) {
+            updateItem();
+        }
+
         // it makes sense to get the cell into its editing state before firing
         // the event to listeners below, so that's what we're doing here
         // by calling super.startEdit().
--- a/javafx-ui-controls/src/javafx/scene/control/TreeCell.java	Fri May 31 12:45:31 2013 +1200
+++ b/javafx-ui-controls/src/javafx/scene/control/TreeCell.java	Mon Jun 03 17:18:14 2013 +1200
@@ -333,6 +333,8 @@
 
     /** {@inheritDoc} */
     @Override public void startEdit() {
+        if (isEditing()) return;
+
         final TreeView<T> tree = getTreeView();
         if (! isEditable() || (tree != null && ! tree.isEditable())) {
 //            if (Logging.getControlsLogger().isLoggable(PlatformLogger.SEVERE)) {
@@ -343,6 +345,8 @@
 //            }
             return;
         }
+
+        updateItem();
         
         // it makes sense to get the cell into its editing state before firing
         // the event to the TreeView below, so that's what we're doing here
@@ -509,7 +513,7 @@
         // the edit mode, then I need to enter the edit mode
         if (match && !editing) {
             startEdit();
-        } else if (match && editing) {
+        } else if (! match && editing) {
             // If my tree item is not the one being edited then I need to cancel
             // the edit. The tricky thing here is that as part of this call
             // I cannot end up calling tree.edit(null) the way that the standard
--- a/javafx-ui-controls/src/javafx/scene/control/TreeTableCell.java	Fri May 31 12:45:31 2013 +1200
+++ b/javafx-ui-controls/src/javafx/scene/control/TreeTableCell.java	Mon Jun 03 17:18:14 2013 +1200
@@ -258,6 +258,8 @@
 
     /** {@inheritDoc} */
     @Override public void startEdit() {
+        if (isEditing()) return;
+
         final TreeTableView<S> table = getTreeTableView();
         final TreeTableColumn<S,T> column = getTableColumn();
         if (! isEditable() ||
@@ -265,7 +267,9 @@
                 (column != null && ! getTableColumn().isEditable())) {
             return;
         }
-        
+
+        updateItem();
+
         // it makes sense to get the cell into its editing state before firing
         // the event to listeners below, so that's what we're doing here
         // by calling super.startEdit().
--- a/javafx-ui-controls/test/com/sun/javafx/scene/control/infrastructure/VirtualFlowTestUtils.java	Fri May 31 12:45:31 2013 +1200
+++ b/javafx-ui-controls/test/com/sun/javafx/scene/control/infrastructure/VirtualFlowTestUtils.java	Mon Jun 03 17:18:14 2013 +1200
@@ -226,6 +226,20 @@
         return getVirtualFlow(control).getCell(index);
     }
 
+    public static IndexedCell getCell(final Control control, final int row, final int column) {
+        IndexedCell rowCell = getVirtualFlow(control).getCell(row);
+        int count = 0;
+        for (Node n : rowCell.getChildrenUnmodifiable()) {
+            if (! (n instanceof IndexedCell)) {
+                continue;
+            }
+            count++;
+            if (count < column) continue;
+            return (IndexedCell) n;
+        }
+        return null;
+    }
+
     public static void assertCallback(final Control control, final int startRow, final int endRow, final Callback<IndexedCell<?>, Void> callback) {
         VirtualFlow<?> flow = getVirtualFlow(control);
 
--- a/javafx-ui-controls/test/javafx/scene/control/CellTest.java	Fri May 31 12:45:31 2013 +1200
+++ b/javafx-ui-controls/test/javafx/scene/control/CellTest.java	Mon Jun 03 17:18:14 2013 +1200
@@ -242,6 +242,9 @@
     }
 
     @Test public void startEditWhenEditableIsTrue() {
+        if ((cell instanceof TableCell)) {
+            ((TableCell) cell).lockItemOnEdit = true;
+        }
         cell.updateItem("Apples", false);
         cell.startEdit();
         assertTrue(cell.isEditing());
@@ -255,6 +258,9 @@
     }
 
     @Test public void startEditWhileAlreadyEditingIsIgnored() {
+        if (cell instanceof TableCell) {
+            ((TableCell) cell).lockItemOnEdit = true;
+        }
         cell.updateItem("Apples", false);
         cell.startEdit();
         cell.startEdit();
--- a/javafx-ui-controls/test/javafx/scene/control/ListViewKeyInputTest.java	Fri May 31 12:45:31 2013 +1200
+++ b/javafx-ui-controls/test/javafx/scene/control/ListViewKeyInputTest.java	Mon Jun 03 17:18:14 2013 +1200
@@ -31,11 +31,16 @@
 
 import java.util.List;
 
+import javafx.beans.property.ReadOnlyStringWrapper;
+import javafx.beans.value.ObservableValue;
+import javafx.event.EventHandler;
 import javafx.scene.Group;
 import javafx.scene.Scene;
 import javafx.scene.input.KeyCode;
 import javafx.stage.Stage;
+import javafx.util.Callback;
 
+import com.sun.javafx.scene.control.infrastructure.VirtualFlowTestUtils;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Test;
@@ -1094,4 +1099,42 @@
         assertEquals(3, fm.getFocusedIndex());
         assertEquals(2, getAnchor());
     }
+
+    private int rt29849_start_count = 0;
+    private int rt29849_cancel_count = 0;
+    @Test public void test_rt29849() {
+        listView.setEditable(true);
+
+        listView.setOnEditStart(new EventHandler<ListView.EditEvent<String>>() {
+            @Override public void handle(ListView.EditEvent<String> t) {
+                rt29849_start_count++;
+            }
+        });
+        listView.setOnEditCancel(new EventHandler<ListView.EditEvent<String>>() {
+            @Override public void handle(ListView.EditEvent<String> t) {
+                rt29849_cancel_count++;
+            }
+        });
+
+        // initially the counts should be zero
+        assertEquals(0, rt29849_start_count);
+        assertEquals(0, rt29849_cancel_count);
+
+        IndexedCell cell = VirtualFlowTestUtils.getCell(listView, 0);
+        assertTrue(cell.isEditable());
+        assertFalse(cell.isEditing());
+        assertEquals(0, cell.getIndex());
+
+        // do an edit, start count should be one, cancel still zero
+        listView.edit(0);
+        assertTrue(cell.isEditing());
+        assertEquals(1, rt29849_start_count);
+        assertEquals(0, rt29849_cancel_count);
+
+        // cancel edit, now both counts should be 1
+        keyboard.doKeyPress(KeyCode.ESCAPE);
+        assertFalse(cell.isEditing());
+        assertEquals(1, rt29849_start_count);
+        assertEquals(1, rt29849_cancel_count);
+    }
 }
--- a/javafx-ui-controls/test/javafx/scene/control/TableViewKeyInputTest.java	Fri May 31 12:45:31 2013 +1200
+++ b/javafx-ui-controls/test/javafx/scene/control/TableViewKeyInputTest.java	Mon Jun 03 17:18:14 2013 +1200
@@ -41,13 +41,17 @@
 
 import java.util.List;
 
+import javafx.beans.property.ReadOnlyStringWrapper;
+import javafx.beans.value.ObservableValue;
 import javafx.event.EventHandler;
 import javafx.scene.Group;
 import javafx.scene.Scene;
 import javafx.scene.input.KeyCode;
 import javafx.scene.input.MouseEvent;
 import javafx.stage.Stage;
+import javafx.util.Callback;
 
+import com.sun.javafx.tk.Toolkit;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Ignore;
@@ -1820,4 +1824,50 @@
         assertEquals(3, fm.getFocusedIndex());
         assertEquals(2, getAnchor().getRow());
     }
+
+    private int rt29849_start_count = 0;
+    private int rt29849_cancel_count = 0;
+    @Test public void test_rt29849() {
+        tableView.setEditable(true);
+        col0.setEditable(true);
+
+        col0.setCellValueFactory(new Callback<TableColumn.CellDataFeatures<String, String>, ObservableValue<String>>() {
+            @Override public ObservableValue<String> call(TableColumn.CellDataFeatures<String, String> param) {
+                return new ReadOnlyStringWrapper("DUMMY TEXT");
+            }
+        });
+
+        col0.setOnEditStart(new EventHandler<TableColumn.CellEditEvent<String, String>>() {
+            @Override public void handle(TableColumn.CellEditEvent<String, String> t) {
+                rt29849_start_count++;
+            }
+        });
+        col0.setOnEditCancel(new EventHandler<TableColumn.CellEditEvent<String, String>>() {
+            @Override public void handle(TableColumn.CellEditEvent<String, String> t) {
+                rt29849_cancel_count++;
+            }
+        });
+
+        // initially the counts should be zero
+        assertEquals(0, rt29849_start_count);
+        assertEquals(0, rt29849_cancel_count);
+
+        TableCell cell = (TableCell)VirtualFlowTestUtils.getCell(tableView, 0, 0);
+        cell.lockItemOnEdit = false;
+        assertTrue(cell.isEditable());
+        assertFalse(cell.isEditing());
+        assertEquals(0, cell.getIndex());
+
+        // do an edit, start count should be one, cancel still zero
+        tableView.edit(0, col0);
+        assertTrue(cell.isEditing());
+        assertEquals(1, rt29849_start_count);
+        assertEquals(0, rt29849_cancel_count);
+
+        // cancel edit, now both counts should be 1
+        keyboard.doKeyPress(KeyCode.ESCAPE);
+        assertFalse(cell.isEditing());
+        assertEquals(1, rt29849_start_count);
+        assertEquals(1, rt29849_cancel_count);
+    }
 }
--- a/javafx-ui-controls/test/javafx/scene/control/TreeTableViewKeyInputTest.java	Fri May 31 12:45:31 2013 +1200
+++ b/javafx-ui-controls/test/javafx/scene/control/TreeTableViewKeyInputTest.java	Mon Jun 03 17:18:14 2013 +1200
@@ -29,7 +29,9 @@
 import com.sun.javafx.scene.control.behavior.TreeTableViewAnchorRetriever;
 import com.sun.javafx.scene.control.infrastructure.KeyEventFirer;
 import com.sun.javafx.scene.control.infrastructure.KeyModifier;
+import com.sun.javafx.scene.control.infrastructure.MouseEventFirer;
 import com.sun.javafx.scene.control.infrastructure.StageLoader;
+import com.sun.javafx.scene.control.infrastructure.VirtualFlowTestUtils;
 import com.sun.javafx.scene.control.skin.TreeTableViewSkin;
 import com.sun.javafx.scene.control.test.Person;
 
@@ -37,14 +39,19 @@
 
 import java.util.List;
 
+import javafx.beans.property.ReadOnlyStringWrapper;
+import javafx.beans.value.ObservableValue;
 import javafx.collections.FXCollections;
 import javafx.collections.ObservableList;
+import javafx.event.EventHandler;
 import javafx.scene.Group;
 import javafx.scene.Scene;
 import javafx.scene.control.cell.TreeItemPropertyValueFactory;
 import javafx.scene.input.KeyCode;
 import javafx.stage.Stage;
+import javafx.util.Callback;
 
+import com.sun.javafx.tk.Toolkit;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Ignore;
@@ -2333,4 +2340,49 @@
         assertEquals(3, fm.getFocusedIndex());
         assertEquals(2, getAnchor().getRow());
     }
+
+    private int rt29849_start_count = 0;
+    private int rt29849_cancel_count = 0;
+    @Test public void test_rt29849() {
+        tableView.setEditable(true);
+        col0.setEditable(true);
+
+        col0.setCellValueFactory(new Callback<TreeTableColumn.CellDataFeatures<String, String>, ObservableValue<String>>() {
+            @Override public ObservableValue<String> call(TreeTableColumn.CellDataFeatures<String, String> param) {
+                return new ReadOnlyStringWrapper("DUMMY TEXT");
+            }
+        });
+
+        col0.setOnEditStart(new EventHandler<TreeTableColumn.CellEditEvent<String, String>>() {
+            @Override public void handle(TreeTableColumn.CellEditEvent<String, String> t) {
+                rt29849_start_count++;
+            }
+        });
+        col0.setOnEditCancel(new EventHandler<TreeTableColumn.CellEditEvent<String, String>>() {
+            @Override public void handle(TreeTableColumn.CellEditEvent<String, String> t) {
+                rt29849_cancel_count++;
+            }
+        });
+
+        // initially the counts should be zero
+        assertEquals(0, rt29849_start_count);
+        assertEquals(0, rt29849_cancel_count);
+
+        IndexedCell cell = VirtualFlowTestUtils.getCell(tableView, 0, 0);
+        assertTrue(cell.isEditable());
+        assertFalse(cell.isEditing());
+        assertEquals(0, cell.getIndex());
+
+        // do an edit, start count should be one, cancel still zero
+        tableView.edit(0, col0);
+        assertTrue(cell.isEditing());
+        assertEquals(1, rt29849_start_count);
+        assertEquals(0, rt29849_cancel_count);
+
+        // cancel edit, now both counts should be 1
+        keyboard.doKeyPress(KeyCode.ESCAPE);
+        assertFalse(cell.isEditing());
+        assertEquals(1, rt29849_start_count);
+        assertEquals(1, rt29849_cancel_count);
+    }
 }
--- a/javafx-ui-controls/test/javafx/scene/control/TreeViewKeyInputTest.java	Fri May 31 12:45:31 2013 +1200
+++ b/javafx-ui-controls/test/javafx/scene/control/TreeViewKeyInputTest.java	Mon Jun 03 17:18:14 2013 +1200
@@ -32,11 +32,16 @@
 
 import java.util.List;
 
+import javafx.beans.property.ReadOnlyStringWrapper;
+import javafx.beans.value.ObservableValue;
+import javafx.event.EventHandler;
 import javafx.scene.Group;
 import javafx.scene.Scene;
 import javafx.scene.input.KeyCode;
 import javafx.stage.Stage;
+import javafx.util.Callback;
 
+import com.sun.javafx.scene.control.infrastructure.VirtualFlowTestUtils;
 import org.junit.After;
 import org.junit.Before;
 import org.junit.Ignore;
@@ -1264,4 +1269,43 @@
         assertEquals(3, fm.getFocusedIndex());
         assertEquals(2, getAnchor());
     }
+
+    private int rt29849_start_count = 0;
+    private int rt29849_cancel_count = 0;
+    @Test public void test_rt29849() {
+        treeView.setEditable(true);
+
+        treeView.setOnEditStart(new EventHandler<TreeView.EditEvent<String>>() {
+            @Override public void handle(TreeView.EditEvent<String> event) {
+                rt29849_start_count++;
+            }
+        });
+        treeView.setOnEditCancel(new EventHandler<TreeView.EditEvent<String>>() {
+            @Override public void handle(TreeView.EditEvent<String> event) {
+                rt29849_cancel_count++;
+            }
+        });
+
+        // initially the counts should be zero
+        assertEquals(0, rt29849_start_count);
+        assertEquals(0, rt29849_cancel_count);
+
+        IndexedCell cell = VirtualFlowTestUtils.getCell(treeView, 0);
+        assertTrue(cell.isEditable());
+        assertFalse(cell.isEditing());
+        assertEquals(0, cell.getIndex());
+
+        // do an edit, start count should be one, cancel still zero
+        treeView.edit(root);
+        assertTrue(cell.isEditing());
+        assertEquals(1, rt29849_start_count);
+        assertEquals(0, rt29849_cancel_count);
+
+        // cancel edit, now both counts should be 1
+//        keyboard.doKeyPress(KeyCode.ESCAPE);
+        treeView.edit(null);
+        assertFalse(cell.isEditing());
+        assertEquals(1, rt29849_start_count);
+        assertEquals(1, rt29849_cancel_count);
+    }
 }