Uploaded image for project: 'JDK'
  1. JDK
  2. JDK-8274433

All Cells: misbehavior of startEdit




      happens if (super) startEdit didn't succeed (== !cell.isEditing)

      - must not fire editStart event
      - must not update control's editing location

      failing tests for ListCell (each failing on the last assert):

          public void testStartEditOffRangeMustNotFireStartEdit() {
              List<EditEvent> events = new ArrayList<>();
              list.addEventHandler(ListView.editStartEvent(), events::add);
              assertFalse("sanity: off-range cell must not be editing", cell.isEditing());
              assertEquals("must not fire editStart", 0, events.size());
          public void testStartEditOffRangeMustNotUpdateEditingLocation() {
              assertFalse("sanity: cell must not be editing if index off range", cell.isEditing());
              assertEquals("editing location", - 1, list.getEditingIndex());
      for list, it sets the editingIndex to the off-range value. For tree-/table, a fix of JDK-8187474 leads to a stackoverflow error. Tree not yet tested, probably similar.

      Problem seems to be the implementation pattern of startEdit (which is basically the same for all concrete cells)

           // backout if not editable for any reason
           if (notEditable ...) return;
           // note: null control and/or other related parties is allowed
           if (eventTarget != null)
           if (editingControl != null)

      that is, both firing and updating control's state will happen even if the cell is not in editing state.

      Checking EditEvent spec: listView.setOnEditStart

      "the EventHandler that will be called when the user begins an edit" implies (for me at least ;) that the edit actually _did_ start.

      Fix would be to backout if cell is not in editing state after calling super (just as the really editable cells in the xx.cell package are doing, see JDK-8188026):

            if (!isEditing()) return;

      P3 because it blocks JDK-8187474


        Issue Links



              fastegal Jeanette Winzenburg
              fastegal Jeanette Winzenburg
              0 Vote for this issue
              3 Start watching this issue