8089514: [TableView, TreeView, ListView, TreeTableView] Clicking outside of the edited cell, node, or entry should commit the value

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/CellBehaviorBase.java

@@ -173,7 +173,7 @@ public void mousePressed(MouseEvent e) {
         } else {
             latePress  = isSelected();
             if (!latePress) {
-                doSelect(e.getX(), e.getY(), e.getButton(), e.getClickCount(),
+                doSelect(e, e.getX(), e.getY(), e.getButton(), e.getClickCount(),
                         e.isShiftDown(), e.isShortcutDown());
             }
         }
@@ -182,7 +182,7 @@ public void mousePressed(MouseEvent e) {
     public void mouseReleased(MouseEvent e) {
         if (latePress) {
             latePress = false;
-            doSelect(e.getX(), e.getY(), e.getButton(), e.getClickCount(),
+            doSelect(e, e.getX(), e.getY(), e.getButton(), e.getClickCount(),
                     e.isShiftDown(), e.isShortcutDown());
         }
     }
@@ -199,7 +199,7 @@ public void mouseDragged(MouseEvent e) {
      *                                                                         *
      **************************************************************************/
 
-    protected void doSelect(final double x, final double y, final MouseButton button,
+    protected void doSelect(MouseEvent e, final double x, final double y, final MouseButton button,
                             final int clickCount, final boolean shiftDown, final boolean shortcutDown) {
         // we update the cell to point to the new tree node
         final T cell = getNode();
@@ -243,7 +243,7 @@ protected void doSelect(final double x, final double y, final MouseButton button
 
         if (button == MouseButton.PRIMARY || (button == MouseButton.SECONDARY && !selected)) {
             if (sm.getSelectionMode() == SelectionMode.SINGLE) {
-                simpleSelect(button, clickCount, shortcutDown);
+                simpleSelect(e, button, clickCount, shortcutDown);
             } else {
                 if (shortcutDown) {
                     if (selected) {
@@ -263,13 +263,13 @@ protected void doSelect(final double x, final double y, final MouseButton button
 
                     fm.focus(index);
                 } else {
-                    simpleSelect(button, clickCount, shortcutDown);
+                    simpleSelect(e, button, clickCount, shortcutDown);
                 }
             }
         }
     }
 
-    protected void simpleSelect(MouseButton button, int clickCount, boolean shortcutDown) {
+    protected void simpleSelect(MouseEvent e, MouseButton button, int clickCount, boolean shortcutDown) {
         final int index = getIndex();
         boolean isAlreadySelected;
 
@@ -288,21 +288,38 @@ protected void simpleSelect(MouseButton button, int clickCount, boolean shortcut
             }
         }
 
-        handleClicks(button, clickCount, isAlreadySelected);
+        doHandleClick(e, button, clickCount, isAlreadySelected);
     }
 
-    protected void handleClicks(MouseButton button, int clickCount, boolean isAlreadySelected) {
+    protected void doHandleClick(MouseEvent e, MouseButton button, int clickCount, boolean isAlreadySelected) {
+        // If not focused yet, we want to shift focus first to the container.
+        if (!getCellContainer().isFocused()) {
+            getCellContainer().requestFocus();
+        }
+
+        // Consume the event if we handled it,
+        // so that the event will not bubble up and shift focus back to the container.
+        if (handleClicks(button, clickCount, isAlreadySelected)) {
+            e.consume();
+        }
+    }
+
+    protected boolean handleClicks(MouseButton button, int clickCount, boolean isAlreadySelected) {
         // handle editing, which only occurs with the primary mouse button
         if (button == MouseButton.PRIMARY) {
             if (clickCount == 1 && isAlreadySelected) {
                 edit(getNode());
+                return true;
             } else if (clickCount == 1) {
-                // cancel editing
+                // stop editing
                 edit(null);
+                return true;
             } else if (clickCount == 2 && getNode().isEditable()) {
                 edit(getNode());
+                return true;
             }
         }
+        return false;
     }
 
     void selectRows(int focusedIndex, int index) {

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TableCellBehaviorBase.java

@@ -33,6 +33,7 @@
 import javafx.scene.control.TablePositionBase;
 import javafx.scene.control.TableSelectionModel;
 import javafx.scene.input.MouseButton;
+import javafx.scene.input.MouseEvent;
 
 public abstract class TableCellBehaviorBase<S, T, TC extends TableColumnBase<S, ?>, C extends IndexedCell<T>> extends CellBehaviorBase<C> {
 
@@ -95,7 +96,7 @@ public TableCellBehaviorBase(C control) {
      **************************************************************************/
 
     @Override
-    protected void doSelect(final double x, final double y, final MouseButton button,
+    protected void doSelect(MouseEvent e, final double x, final double y, final MouseButton button,
                           final int clickCount, final boolean shiftDown, final boolean shortcutDown) {
         // Note that table.select will reset selection
         // for out of bounds indexes. So, need to check
@@ -146,7 +147,7 @@ protected void doSelect(final double x, final double y, final MouseButton button
         // what modifiers the user held down as they released the mouse.
         if (button == MouseButton.PRIMARY || (button == MouseButton.SECONDARY && !selected)) {
             if (sm.getSelectionMode() == SelectionMode.SINGLE) {
-                simpleSelect(button, clickCount, shortcutDown);
+                simpleSelect(e, button, clickCount, shortcutDown);
             } else {
                 if (shortcutDown) {
                     if (selected) {
@@ -195,14 +196,14 @@ protected void doSelect(final double x, final double y, final MouseButton button
                     // return selection back to the focus owner
                     // focus(anchor.getRow(), tableColumn);
                 } else {
-                    simpleSelect(button, clickCount, shortcutDown);
+                    simpleSelect(e, button, clickCount, shortcutDown);
                 }
             }
         }
     }
 
     @Override
-    protected void simpleSelect(MouseButton button, int clickCount, boolean shortcutDown) {
+    protected void simpleSelect(MouseEvent e, MouseButton button, int clickCount, boolean shortcutDown) {
         final TableSelectionModel<S> sm = getSelectionModel();
         boolean isAlreadySelected;
 
@@ -223,7 +224,7 @@ protected void simpleSelect(MouseButton button, int clickCount, boolean shortcut
             }
         }
 
-        handleClicks(button, clickCount, isAlreadySelected);
+        doHandleClick(e, button, clickCount, isAlreadySelected);
     }
 
     private int getColumn() {

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TableRowBehaviorBase.java

@@ -77,7 +77,7 @@ public TableRowBehaviorBase(T control) {
      *                                                                         *
      **************************************************************************/
 
-    @Override protected void doSelect(final double x, final double y, final MouseButton button,
+    @Override protected void doSelect(MouseEvent e, final double x, final double y, final MouseButton button,
                    final int clickCount, final boolean shiftDown, final boolean shortcutDown) {
         final Control table = getCellContainer();
         if (table == null) return;
@@ -112,11 +112,11 @@ public TableRowBehaviorBase(T control) {
                     final int anchorRow = anchor.getRow();
                     selectRows(anchorRow, index);
                 } else {
-                    simpleSelect(button, clickCount, shortcutDown);
+                    simpleSelect(e, button, clickCount, shortcutDown);
                 }
             }
         } else {
-            simpleSelect(button, clickCount, shortcutDown);
+            simpleSelect(e, button, clickCount, shortcutDown);
         }
     }
 

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TreeCellBehavior.java

@@ -32,6 +32,7 @@
 import javafx.scene.control.TreeItem;
 import javafx.scene.control.TreeView;
 import javafx.scene.input.MouseButton;
+import javafx.scene.input.MouseEvent;
 
 public class TreeCellBehavior<T> extends CellBehaviorBase<TreeCell<T>> {
 
@@ -75,23 +76,27 @@ protected void edit(TreeCell<T> cell) {
     }
 
     @Override
-    protected void handleClicks(MouseButton button, int clickCount, boolean isAlreadySelected) {
+    protected boolean handleClicks(MouseButton button, int clickCount, boolean isAlreadySelected) {
         // handle editing, which only occurs with the primary mouse button
         TreeItem<T> treeItem = getNode().getTreeItem();
         if (button == MouseButton.PRIMARY) {
             if (clickCount == 1 && isAlreadySelected) {
                 edit(getNode());
+                return true;
             } else if (clickCount == 1) {
-                // cancel editing
+                // stop editing
                 edit(null);
+                return true;
             } else if (clickCount == 2 && treeItem.isLeaf()) {
                 // attempt to edit
                 edit(getNode());
+                return true;
             } else if (clickCount % 2 == 0) {
                 // try to expand/collapse branch tree item
                 treeItem.setExpanded(! treeItem.isExpanded());
             }
         }
+        return false;
     }
 
     @Override protected boolean handleDisclosureNode(double x, double y) {

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TreeTableCellBehavior.java

@@ -33,6 +33,7 @@
 import javafx.scene.control.TreeTableColumn;
 import javafx.scene.control.TreeTableView;
 import javafx.scene.input.MouseButton;
+import javafx.scene.input.MouseEvent;
 
 /**
  */
@@ -140,22 +141,26 @@ public TreeTableCellBehavior(TreeTableCell<S,T> control) {
     }
 
     @Override
-    protected void handleClicks(MouseButton button, int clickCount, boolean isAlreadySelected) {
+    protected boolean handleClicks(MouseButton button, int clickCount, boolean isAlreadySelected) {
         // handle editing, which only occurs with the primary mouse button
         TreeItem<S> treeItem = getNode().getTableRow().getTreeItem();
         if (button == MouseButton.PRIMARY) {
             if (clickCount == 1 && isAlreadySelected) {
                 edit(getNode());
+                return true;
             } else if (clickCount == 1) {
-                // cancel editing
+                // stop editing
                 edit(null);
+                return true;
             } else if (clickCount == 2 && treeItem.isLeaf()) {
                 // attempt to edit
                 edit(getNode());
+                return true;
             } else if (clickCount % 2 == 0) {
                 // try to expand/collapse branch tree item
                 treeItem.setExpanded(! treeItem.isExpanded());
             }
         }
+        return false;
     }
 }

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TreeTableRowBehavior.java

@@ -33,6 +33,7 @@
 import javafx.scene.control.TreeTableRow;
 import javafx.scene.control.TreeTableView;
 import javafx.scene.input.MouseButton;
+import javafx.scene.input.MouseEvent;
 
 public class TreeTableRowBehavior<T> extends TableRowBehaviorBase<TreeTableRow<T>> {
 
@@ -77,22 +78,26 @@ public TreeTableRowBehavior(TreeTableRow<T> control) {
     }
 
     @Override
-    protected void handleClicks(MouseButton button, int clickCount, boolean isAlreadySelected) {
+    protected boolean handleClicks(MouseButton button, int clickCount, boolean isAlreadySelected) {
         // handle editing, which only occurs with the primary mouse button
         TreeItem<T> treeItem = getNode().getTreeItem();
         if (button == MouseButton.PRIMARY) {
             if (clickCount == 1 && isAlreadySelected) {
                 edit(getNode());
+                return true;
             } else if (clickCount == 1) {
-                // cancel editing
+                // stop editing
                 edit(null);
+                return true;
             } else if (clickCount == 2 && treeItem.isLeaf()) {
                 // attempt to edit
                 edit(getNode());
+                return true;
             } else if (clickCount % 2 == 0) {
                 // try to expand/collapse branch tree item
                 treeItem.setExpanded(! treeItem.isExpanded());
             }
         }
+        return false;
     }
 }

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

@@ -25,8 +25,6 @@
 
 package javafx.scene.control;
 
-import javafx.beans.InvalidationListener;
-import javafx.beans.Observable;
 import javafx.beans.property.BooleanProperty;
 import javafx.beans.property.ObjectProperty;
 import javafx.beans.property.SimpleBooleanProperty;
@@ -353,26 +351,39 @@ public Cell() {
         ((StyleableProperty<Boolean>)focusTraversableProperty()).applyStyle(null, Boolean.FALSE);
         getStyleClass().addAll(DEFAULT_STYLE_CLASS);
 
-        /**
-         * Indicates whether or not this cell has focus. For example, a
+        /*
+         * Indicates whether this cell has focus. For example, a
          * ListView defines zero or one cell as being the "focused" cell. This cell
          * would have focused set to true.
          */
-        super.focusedProperty().addListener(new InvalidationListener() {
-            @Override public void invalidated(Observable property) {
-                pseudoClassStateChanged(PSEUDO_CLASS_FOCUSED, isFocused()); // TODO is this necessary??
-
-                // The user has shifted focus, so we should cancel the editing on this cell
-                if (!isFocused() && isEditing()) {
-                    cancelEdit();
-                }
+        focusedProperty().addListener(_ -> {
+            pseudoClassStateChanged(PSEUDO_CLASS_FOCUSED, isFocused()); // TODO is this necessary??
+
+            // The user has shifted focus, so we should stop the editing on this cell.
+            if (!isFocused() && isEditing()) {
+                stopEdit();
             }
         });
 
         // initialize default pseudo-class state
         pseudoClassStateChanged(PSEUDO_CLASS_EMPTY, true);
     }
 
+    /**
+     * Stops the edit operation of the cell.
+     * This method is called when another cell is edited or the focus changed.
+     * <p>
+     * The default behavior is to cancel the edit.
+     * This method is meant to be subclassed in case the default behavior is not enough.
+     * For example, subclasses decide to rather commit the edit operation instead of cancelling.
+     */
+    public void stopEdit() {
+        if (!isEditing()) {
+            return;
+        }
+
+        cancelEdit();
+    }
 
 
     /* *************************************************************************

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

@@ -592,23 +592,28 @@ private void updateEditing() {
         if (match && !editing) {
             startEdit();
         } else if (!match && editing) {
-            // If my index 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 list.edit(-1) the way that the standard
-            // cancelEdit method would do. Yet, I need to call cancelEdit
-            // so that subclasses which override cancelEdit can execute. So,
-            // I have to use a kind of hacky flag workaround.
-            try {
-                // try-finally to make certain that the flag is reliably reset to true
-                updateEditingIndex = false;
-                cancelEdit();
-            } finally {
-                updateEditingIndex = true;
-            }
+            doStopEdit();
         }
     }
 
-
+    /**
+     * Stops the edit operation.
+     * If not overwritten, will cancel the edit without changing control'sediting state.
+     */
+    private void doStopEdit() {
+        // If my index 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 list.edit(-1) the way that the standard
+        // cancelEdit method would do. Yet, I need to call cancelEdit
+        // so that subclasses which override cancelEdit can execute. So,
+        // I have to use a kind of hacky flag workaround.
+        try {
+            updateEditingIndex = false;
+            stopEdit();
+        } finally {
+            updateEditingIndex = true;
+        }
+    }
 
     /* *************************************************************************
      *                                                                         *
@@ -654,4 +659,3 @@ public void executeAccessibleAction(AccessibleAction action, Object... parameter
         }
     }
 }
-