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

All Cells: misbehavior of startEdit

    XMLWordPrintable

    Details

      Description

      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):

          @Test
          public void testStartEditOffRangeMustNotFireStartEdit() {
              list.setEditable(true);
              cell.updateListView(list);
              cell.updateIndex(list.getItems().size());
              List<EditEvent> events = new ArrayList<>();
              list.addEventHandler(ListView.editStartEvent(), events::add);
              cell.startEdit();
              assertFalse("sanity: off-range cell must not be editing", cell.isEditing());
              assertEquals("must not fire editStart", 0, events.size());
          }
          
          @Test
          public void testStartEditOffRangeMustNotUpdateEditingLocation() {
              list.setEditable(true);
              cell.updateListView(list);
              cell.updateIndex(list.getItems().size());
              cell.startEdit();
              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
           super.startEdit();
           if (eventTarget != null)
               fire(editStart)
           if (editingControl != null)
              editingControl.edit(cellLocation)

      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):

            ....
            super.startEdit();
            if (!isEditing()) return;
            ...

      P3 because it blocks JDK-8187474

        Attachments

          Issue Links

            Activity

              People

              Assignee:
              fastegal Jeanette Winzenburg
              Reporter:
              fastegal Jeanette Winzenburg
              Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved: