Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

8274433: All Cells: misbehavior of startEdit #636

Closed
wants to merge 2 commits into from
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
@@ -368,6 +368,7 @@ public void changed(
// by calling super.startEdit().
super.startEdit();

if (!isEditing()) return;
// Inform the ListView of the edit starting.
if (list != null) {
list.fireEvent(new ListView.EditEvent<T>(list,
@@ -331,6 +331,7 @@ private void setTableView(TableView<S> value) {
// by calling super.startEdit().
super.startEdit();

if (!isEditing()) return;
editingCellAtStartEdit = new TablePosition<>(table, getIndex(), column);
if (column != null) {
CellEditEvent<S,?> editEvent = new CellEditEvent<>(
@@ -376,6 +376,7 @@ public String getName() {
// by calling super.startEdit().
super.startEdit();

if (!isEditing()) return;
// Inform the TreeView of the edit starting.
if (tree != null) {
tree.fireEvent(new TreeView.EditEvent<T>(tree,
@@ -348,6 +348,7 @@ private void setTreeTableView(TreeTableView<S> value) {
// by calling super.startEdit().
super.startEdit();

if (!isEditing()) return;
editingCellAtStartEdit = new TreeTablePosition<>(table, getIndex(), column);
if (column != null) {
CellEditEvent<S, T> editEvent = new CellEditEvent<>(
@@ -852,6 +852,28 @@ public void testEditCancelEventAfterRemoveEditingItem() {
assertEquals("editing location of cancel event", editingIndex, events.get(0).getIndex());
}

@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: off-range cell must not be editing", cell.isEditing());
assertEquals("list editing location must not be updated", -1, list.getEditingIndex());
}


// When the list view item's change and affects a cell that is editing, then what?
// When the list cell's index is changed while it is editing, then what?
@@ -31,6 +31,7 @@

import org.junit.After;
import org.junit.Before;
import org.junit.Ignore;
import org.junit.Test;

import com.sun.javafx.tk.Toolkit;
@@ -701,6 +702,32 @@ public void testEditCommitNullTableNullColumn() {
cell.commitEdit("edited");
}

@Test
public void testStartEditOffRangeMustNotFireStartEdit() {
setupForEditing();
int editingRow = table.getItems().size();
cell.updateIndex(editingRow);
List<CellEditEvent<?, ?>> events = new ArrayList<>();
editingColumn.addEventHandler(TableColumn.editStartEvent(), events::add);
cell.startEdit();
assertFalse("sanity: off-range cell must not be editing", cell.isEditing());
assertEquals("must not fire editStart", 0, events.size());
}

/**
* Note: this is a false green until JDK-8187474 (update control editing location) is fixed
*/
@Ignore("JDK-8187474")
@Test
public void testStartEditOffRangeMustNotUpdateEditingLocation() {
setupForEditing();
int editingRow = table.getItems().size();
cell.updateIndex(editingRow);
cell.startEdit();
assertFalse("sanity: off-range cell must not be editing", cell.isEditing());
assertNull("table editing location must not be updated", table.getEditingCell());
}

//--------- test the test setup

@Test
@@ -844,6 +844,33 @@ public void testEditCommitMemoryLeakAfterRemoveEditingItem() {
assertEquals("treeItem must be gc'ed", null, itemRef.get());
}

@Test
public void testStartEditOffRangeMustNotFireStartEdit() {
tree.setEditable(true);
cell.updateTreeView(tree);
cell.updateIndex(tree.getExpandedItemCount());
List<EditEvent<?>> events = new ArrayList<>();
tree.addEventHandler(TreeView.editStartEvent(), events::add);
cell.startEdit();
assertFalse("sanity: off-range cell must not be editing", cell.isEditing());
assertEquals("cell must not fire editStart if not editing", 0, events.size());
}

/**
* Note: this is a false green until JDK-8187474 (update control editing location) is fixed
*/
@Ignore("JDK-8187474")
@Test
public void testStartEditOffRangeMustNotUpdateEditingLocation() {
tree.setEditable(true);
cell.updateTreeView(tree);
cell.updateIndex(tree.getExpandedItemCount());
cell.startEdit();
assertFalse("sanity: off-range cell must not be editing", cell.isEditing());
assertNull("tree editing location must not be updated", tree.getEditingItem());
}


// When the tree view item's change and affects a cell that is editing, then what?
// When the tree cell's index is changed while it is editing, then what?

@@ -1027,6 +1027,31 @@ public void testEditCommitNullTableNullColumn() {
cell.commitEdit("edited");
}

@Test
public void testStartEditOffRangeMustNotFireStartEdit() {
setupForEditing();
cell.updateIndex(tree.getExpandedItemCount());
List<CellEditEvent<?, ?>> events = new ArrayList<>();
editingColumn.addEventHandler(TreeTableColumn.editStartEvent(), events::add);
cell.startEdit();
assertFalse("sanity: off-range cell must not be editing", cell.isEditing());
assertEquals("cell must not fire editStart if not editing", 0, events.size());
}

/**
* Note: this would be a false green until JDK-8187474 (update control editing location) is fixed
*/
@Ignore("JDK-8187474")
@Test
public void testStartEditOffRangeMustNotUpdateEditingLocation() {
setupForEditing();
cell.updateIndex(tree.getExpandedItemCount());
cell.startEdit();
assertFalse("sanity: off-range cell must not be editing", cell.isEditing());
assertNull("treetable editing location must not be updated", tree.getEditingCell());
}


//--------- test the test setup

@Test