8187314: All Cells: must show backing data always

Reviewed-by: angorya, aghaisas

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2010, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2010, 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
@@ -404,8 +404,8 @@ public void changed(
                     list.getEditingIndex()));
         }
 
-        // update the item within this cell, so that it represents the new value
-        updateItem(newValue, false);
+        // Update the item within this cell, so that it represents the new value
+        updateItem(-1);
 
         if (list != null) {
             // reset the editing index on the ListView. This must come after the

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

@@ -367,8 +367,8 @@ TablePosition<S, T> getEditingCellAtStartEdit() {
             Event.fireEvent(getTableColumn(), editEvent);
         }
 
-        // update the item within this cell, so that it represents the new value
-        updateItem(newValue, false);
+        // Update the item within this cell, so that it represents the new value
+        updateItem(-1);
 
         if (table != null) {
             // reset the editing cell on the TableView

modules/javafx.controls/src/main/java/javafx/scene/control/TreeCell.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
@@ -414,9 +414,8 @@ 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
-        updateItem(newValue, false);
+        // Update the item within this cell, so that it represents the new value
+        updateItem(-1);
 
         if (tree != null) {
             // reset the editing item in the TreetView

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

@@ -384,8 +384,8 @@ TreeTablePosition<S, T> getEditingCellAtStartEdit() {
             Event.fireEvent(getTableColumn(), editEvent);
         }
 
-        // update the item within this cell, so that it represents the new value
-        updateItem(newValue, false);
+        // Update the item within this cell, so that it represents the new value
+        updateItem(-1);
 
         if (table != null) {
             // reset the editing cell on the TableView

modules/javafx.controls/src/test/java/test/javafx/scene/control/ListCellTest.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
@@ -1106,6 +1106,30 @@ public void testMisbehavingCancelEditTerminatesEdit() {
         }
     }
 
+    /**
+     * See also: <a href="https://bugs.openjdk.org/browse/JDK-8187314">JDK-8187314</a>.
+     */
+    @Test
+    public void testEditCommitValueChangeIsReflectedInCell() {
+        list.setEditable(true);
+
+        cell.updateListView(list);
+        list.setOnEditCommit(event -> {
+            assertEquals("ABCDEF", event.getNewValue());
+            // Change the underlying item.
+            model.set(0, "ABCDEF [Changed]");
+        });
+
+        cell.updateIndex(0);
+
+        assertEquals("Apples", cell.getItem());
+
+        cell.startEdit();
+        cell.commitEdit("ABCDEF");
+
+        assertEquals("ABCDEF [Changed]", cell.getItem());
+    }
+
     public static class MisbehavingOnCancelListCell<T> extends ListCell<T> {
         @Override
         public void cancelEdit() {

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

@@ -902,6 +902,29 @@ public void testMisbehavingCancelEditTerminatesEdit() {
         }
     }
 
+    /**
+     * See also: <a href="https://bugs.openjdk.org/browse/JDK-8187314">JDK-8187314</a>.
+     */
+    @Test
+    public void testEditCommitValueChangeIsReflectedInCell() {
+        setupForEditing();
+        editingColumn.setCellValueFactory(cc -> new SimpleObjectProperty<>(cc.getValue()));
+        editingColumn.setOnEditCommit(event -> {
+            assertEquals("ABCDEF", event.getNewValue());
+            // Change the underlying item.
+            model.set(0, "ABCDEF [Changed]");
+        });
+
+        cell.updateIndex(0);
+
+        assertEquals("Four", cell.getItem());
+
+        cell.startEdit();
+        cell.commitEdit("ABCDEF");
+
+        assertEquals("ABCDEF [Changed]", cell.getItem());
+    }
+
     public static class MisbehavingOnCancelTableCell<S, T> extends TableCell<S, T> {
 
         @Override

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

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2013, 2022, Oracle and/or its affiliates. All rights reserved.
+ * Copyright (c) 2013, 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
@@ -927,7 +927,6 @@ public void testDoNothingCommitHandlerDoesNotUpdateData() {
         assertEquals("edited value must not be committed", oldValue, editingItem.getValue());
     }
 
-    @Ignore("JDK-8187314")
     @Test
     public void testDoNothingCommitHandlerDoesNotUpdateCell() {
         TreeCell<String> cell = TextFieldTreeCell.forTreeView().call(tree);
@@ -1037,4 +1036,28 @@ public void testTreeCellHeights() {
                 cell.maxHeight(-1), 1);
     }
 
+    /**
+     * See also: <a href="https://bugs.openjdk.org/browse/JDK-8187314">JDK-8187314</a>.
+     */
+    @Test
+    public void testEditCommitValueChangeIsReflectedInCell() {
+        tree.setEditable(true);
+
+        cell.updateTreeView(tree);
+        tree.setOnEditCommit(event -> {
+            assertEquals("ABCDEF", event.getNewValue());
+            // Change the underlying item.
+            root.setValue("ABCDEF [Changed]");
+        });
+
+        cell.updateIndex(0);
+
+        assertEquals("Root", cell.getItem());
+
+        cell.startEdit();
+        cell.commitEdit("ABCDEF");
+
+        assertEquals("ABCDEF [Changed]", cell.getItem());
+    }
+
 }

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

@@ -1214,6 +1214,29 @@ public void testMisbehavingCancelEditTerminatesEdit() {
         }
     }
 
+    /**
+     * See also: <a href="https://bugs.openjdk.org/browse/JDK-8187314">JDK-8187314</a>.
+     */
+    @Test
+    public void testEditCommitValueChangeIsReflectedInCell() {
+        setupForEditing();
+        editingColumn.setCellValueFactory(cc -> new SimpleObjectProperty<>(cc.getValue().getValue()));
+        editingColumn.setOnEditCommit(event -> {
+            assertEquals("ABCDEF", event.getNewValue());
+            // Change the underlying item.
+            root.setValue("ABCDEF [Changed]");
+        });
+
+        cell.updateIndex(0);
+
+        assertEquals("Root", cell.getItem());
+
+        cell.startEdit();
+        cell.commitEdit("ABCDEF");
+
+        assertEquals("ABCDEF [Changed]", cell.getItem());
+    }
+
     public static class MisbehavingOnCancelTreeTableCell<S, T> extends TreeTableCell<S, T> {
 
         @Override