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

TreeView: must have default editCommit handler

    Details

    • Type: Bug
    • Status: Open
    • Priority: P3
    • Resolution: Unresolved
    • Affects Version/s: 9
    • Fix Version/s: tbd
    • Component/s: javafx
    • Environment:

      java9-ea-180

      Description

      List/TableColumn have default editCommit handlers, TreeView has none (see failing test below), introducing an inconsistency and breaking its spec:

      "By default the TreeView edit commit handler is non-null with a default handler that attempts to overwrite the property value for the item in the currently-being-edited row."

      Not having one might be the reason (wildly speculating ;) for
      TreeCell to illegally take over the task in commitEdit and change the backing data (reported as JDK-8187309)

      The fix for both is rather simple (and low risk, I think):

      1. add a default handler like f.i.

              treeView.setOnEditCommit(e -> {
                  TreeItem editItem = e.getTreeItem();
                  editItem.setValue(e.getNewValue());
              });

      2. remove interfering code in treeCell.commitEdit

      // current code
              // update the item within this cell, so that it represents the new value
              // no, that's is wrong!
      // if (treeItem != null) {
      // treeItem.setValue(newValue);
      // updateTreeItem(treeItem);
      // updateItem(newValue, false);
      // }

      // replace with:
              updateItem(newValue, false);



      The tests (pass for List/TableColumn, fail for Tree)

          /**
           * Test default edit handlers: expected none for start/cancel,
           * default that commits
           *
           * Here: Table
           */
          @ConditionalIgnore (condition = IgnoreTableEdit.class)
          @Test
          public void testTableEditHandler() {
              TableView<TableColumn> table = createEditableTable();
              new StageLoader(table);
              TableColumn control = table.getColumns().get(0);
              assertNull(control.getOnEditCancel());
              assertNull(control.getOnEditStart());
              assertNotNull("listView must have default commit handler", control.getOnEditCommit());
          }
          
          /**
           * Test default edit handlers: expected none for start/cancel,
           * default that commits
           *
           * Here: List
           */
          @ConditionalIgnore (condition = IgnoreListEdit.class)
          @Test
          public void testListEditHandler() {
              ListView<String> control = createEditableList();
              new StageLoader(control);
              assertNull(control.getOnEditCancel());
              assertNull(control.getOnEditStart());
              assertNotNull("listView must have default commit handler", control.getOnEditCommit());
          }
          
          /**
           * Test default edit handlers: expected none for start/cancel,
           * default that commits
           *
           * Here: Tree - fails, probably reason for cell taking over itself
           * (https://bugs.openjdk.java.net/browse/JDK-8187309)
           */
          @ConditionalIgnore (condition = IgnoreTreeEdit.class)
          @Test
          public void testTreeEditHandler() {
              TreeView<String> control = createEditableTree();
              new StageLoader(control);
              assertNull(control.getOnEditCancel());
              assertNull(control.getOnEditStart());
              assertNotNull("treeView must have default commit handler", control.getOnEditCommit());
          }

        Attachments

          Issue Links

            Activity

              People

              • Assignee:
                aghaisas Ajit Ghaisas
                Reporter:
                fastegal Jeanette Winzenburg
              • Votes:
                0 Vote for this issue
                Watchers:
                2 Start watching this issue

                Dates

                • Created:
                  Updated: