JDK-8313799: Remove lockItemOnEdit flag from (Tree)TableCell

modules/javafx.controls/src/main/java/javafx/scene/control/TableCell.java

@@ -100,8 +100,6 @@ public TableCell() {
      *                                                                         *
      **************************************************************************/
 
-    // package for testing
-    boolean lockItemOnEdit = false;
 
 
     /* *************************************************************************
@@ -319,12 +317,7 @@ TablePosition<S, T> getEditingCellAtStartEdit() {
             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(-1);
-        }
+        updateItem(-1);
 
         // 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

modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableCell.java

@@ -98,8 +98,6 @@ public TreeTableCell() {
      *                                                                         *
      **************************************************************************/
 
-    // package for testing
-    boolean lockItemOnEdit = false;
 
 
 
@@ -336,12 +334,7 @@ TreeTablePosition<S, T> getEditingCellAtStartEdit() {
             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(-1);
-        }
+        updateItem(-1);
 
         // 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

modules/javafx.controls/src/shims/java/javafx/scene/control/TableCellShim.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -26,13 +26,37 @@
 
 public class TableCellShim<S,T> extends TableCell<S,T> {
 
+    /**
+     * Flag which is only used in conjunction with {@link #lockItemOnStartEdit} to lock the item when
+     * the {@link #startEdit()} method is called.
+     */
+    private boolean isStartEdit = false;
+    /**
+     * Flag to lock the item value when an edit process is started.
+     * While normally the {@link #updateItem(Object, boolean)} will change the underlying item,
+     * when locked the item will not be changed.
+     */
+    private boolean lockItemOnStartEdit = false;
+
+    @Override
+    public void startEdit() {
+        isStartEdit = true;
+        super.startEdit();
+    }
+
     @Override
     public void updateItem(T item, boolean empty) {
+        // startEdit() was called and wants to update the cell. When locked, we will ignore the update request.
+        if (lockItemOnStartEdit && isStartEdit) {
+            isStartEdit = false;
+            return;
+        }
+
         super.updateItem(item, empty);
     }
 
-    public static <S, T> void set_lockItemOnEdit(TableCell<S, T> tc, boolean b) {
-        tc.lockItemOnEdit = b;
+    public void setLockItemOnStartEdit(boolean lockItemOnEdit) {
+        this.lockItemOnStartEdit = lockItemOnEdit;
     }
 
     public static <S, T> TablePosition<S, T> getEditingCellAtStartEdit(TableCell<S, T> cell) {

modules/javafx.controls/src/shims/java/javafx/scene/control/TreeTableCellShim.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2015, 2021, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2015, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -26,13 +26,37 @@
 
 public class TreeTableCellShim<S,T> extends TreeTableCell<S,T> {
 
+    /**
+     * Flag which is only used in conjunction with {@link #lockItemOnStartEdit} to lock the item when
+     * the {@link #startEdit()} method is called.
+     */
+    private boolean isStartEdit = false;
+    /**
+     * Flag to lock the item value when an edit process is started.
+     * While normally the {@link #updateItem(Object, boolean)} will change the underlying item,
+     * when locked the item will not be changed.
+     */
+    private boolean lockItemOnStartEdit = false;
+
+    @Override
+    public void startEdit() {
+        isStartEdit = true;
+        super.startEdit();
+    }
+
     @Override
     public void updateItem(T item, boolean empty) {
+        // startEdit() was called and wants to update the cell. When locked, we will ignore the update request.
+        if (lockItemOnStartEdit && isStartEdit) {
+            isStartEdit = false;
+            return;
+        }
+
         super.updateItem(item, empty);
     }
 
-    public static <S, T> void set_lockItemOnEdit(TreeTableCell<S, T> tc, boolean b) {
-        tc.lockItemOnEdit = b;
+    public void setLockItemOnStartEdit(boolean lockItemOnEdit) {
+        this.lockItemOnStartEdit = lockItemOnEdit;
     }
 
     public static <S, T> TreeTablePosition<S, T> getEditingCellAtStartEdit(TreeTableCell<S, T> cell) {

modules/javafx.controls/src/test/java/test/javafx/scene/control/CellTest.java

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2011, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2011, 2023, Oracle and/or its affiliates. All rights reserved.
  * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
  *
  * This code is free software; you can redistribute it and/or modify it
@@ -66,10 +66,12 @@ public class CellTest {
                 {Cell.class},
                 {ListCell.class},
                 {TableRow.class},
-                {TableCell.class},
+                // Note: We use the shim here, so we can lock the item. The behaviour is the same otherwise.
+                {TableCellShim.class},
                 {TreeCell.class},
                 {TreeTableRow.class},
-                {TreeTableCell.class}
+                // Note: We use the shim here, so we can lock the item.  The behaviour is the same otherwise.
+                {TreeTableCellShim.class}
         });
     }
 
@@ -90,12 +92,12 @@ public CellTest(Class type) {
             TableRow tableRow = new TableRow();
             CellShim.updateItem(tableRow, "TableRow", false);
             ((TableCell)cell).updateTableRow(tableRow);
-            TableCellShim.set_lockItemOnEdit((TableCell)cell, true);
+            ((TableCellShim)cell).setLockItemOnStartEdit(true);
         } else if (cell instanceof TreeTableCell) {
             TreeTableRow tableRow = new TreeTableRow();
             CellShim.updateItem(tableRow, "TableRow", false);
             ((TreeTableCell)cell).updateTableRow(tableRow);
-            TreeTableCellShim.set_lockItemOnEdit((TreeTableCell)cell, true);
+            ((TreeTableCellShim)cell).setLockItemOnStartEdit(true);
         }
     }
 
@@ -274,9 +276,6 @@ public void selectingAnEmptyCellResultsInNoChange() {
     }
 
     @Test public void startEditWhenEditableIsTrue() {
-        if ((cell instanceof TableCell)) {
-            TableCellShim.set_lockItemOnEdit((TableCell) cell, true);
-        }
         CellShim.updateItem(cell, "Apples", false);
         cell.startEdit();
         assertTrue(cell.isEditing());
@@ -290,9 +289,6 @@ public void selectingAnEmptyCellResultsInNoChange() {
     }
 
     @Test public void startEditWhileAlreadyEditingIsIgnored() {
-        if (cell instanceof TableCell) {
-            TableCellShim.set_lockItemOnEdit((TableCell) cell, true);
-        }
         CellShim.updateItem(cell, "Apples", false);
         cell.startEdit();
         cell.startEdit();

modules/javafx.controls/src/test/java/test/javafx/scene/control/TableCellTest.java

@@ -59,7 +59,7 @@
 /**
  */
 public class TableCellTest {
-    private TableCell<String,String> cell;
+    private TableCellShim<String, String> cell;
     private TableView<String> table;
     private TableColumn<String, String> editingColumn;
     private TableRow<String> row;
@@ -75,7 +75,7 @@ public class TableCellTest {
             }
         });
 
-        cell = new TableCell<>();
+        cell = new TableCellShim<>();
         model = FXCollections.observableArrayList("Four", "Five", "Fear"); // "Flop", "Food", "Fizz"
         table = new TableView<>(model);
         editingColumn = new TableColumn<>("TEST");
@@ -852,7 +852,7 @@ private void setupForcedEditing(TableView table, TableColumn editingColumn) {
          }
          if (editingColumn != null ) cell.updateTableColumn(editingColumn);
          // force into editable state (not empty)
-         TableCellShim.set_lockItemOnEdit(cell, true);
+         cell.setLockItemOnStartEdit(true);
          CellShim.updateItem(cell, "something", false);
      }
 

modules/javafx.controls/src/test/java/test/javafx/scene/control/TableViewKeyInputTest.java

@@ -1995,7 +1995,6 @@ public void testEnterOnFocusedRowDoesNotThrowNPE() {
         assertEquals(0, rt29849_cancel_count);
 
         TableCell cell = (TableCell)VirtualFlowTestUtils.getCell(tableView, 0, 0);
-        TableCellShim.set_lockItemOnEdit(cell, false);
         assertTrue(cell.isEditable());
         assertFalse(cell.isEditing());
         assertEquals(0, cell.getIndex());

modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeTableCellTest.java

@@ -57,7 +57,7 @@
 import static org.junit.Assert.*;
 
 public class TreeTableCellTest {
-    private TreeTableCell<String, String> cell;
+    private TreeTableCellShim<String, String> cell;
     private TreeTableView<String> tree;
     private TreeTableRow<String> row;
 
@@ -83,7 +83,7 @@ public class TreeTableCellTest {
             }
         });
 
-        cell = new TreeTableCell<>();
+        cell = new TreeTableCellShim<>();
 
         root = new TreeItem<>(ROOT);
         apples = new TreeItem<>(APPLES);
@@ -1163,7 +1163,7 @@ private void setupForcedEditing(TreeTableView table, TreeTableColumn editingColu
          }
          if (editingColumn != null ) cell.updateTableColumn(editingColumn);
          // force into editable state (not empty)
-         TreeTableCellShim.set_lockItemOnEdit(cell, true);
+         cell.setLockItemOnStartEdit(true);
          CellShim.updateItem(cell, "something", false);
      }