Skip to content
Permalink
Browse files
8264127: ListCell editing status is true, when index changes while ed…
…iting

Reviewed-by: fastegal, aghaisas
  • Loading branch information
FlorianKirmaier authored and Jeanette Winzenburg committed May 22, 2021
1 parent 5843910 commit 240d28ff4c73777f57fdf88250cceb601e5c18c1
@@ -337,6 +337,7 @@ public void changed(
updateItem(oldIndex);
updateSelection();
updateFocus();
updateEditing();
}
}

@@ -539,22 +540,22 @@ private void updateEditing() {
final ListView<T> list = getListView();
final int editIndex = list == null ? -1 : list.getEditingIndex();
final boolean editing = isEditing();

// Check that the list is specified, and my index is not -1
if (index != -1 && list != null) {
// If my index is the index being edited and I'm not currently in
// the edit mode, then I need to enter the edit mode
if (index == editIndex && !editing) {
startEdit();
} else if (index != editIndex && 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.
final boolean match = (list != null) && (index != -1) && (index == editIndex);

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;
}
}
@@ -35,9 +35,13 @@
import javafx.scene.control.ListCell;
import javafx.scene.control.ListCellShim;
import javafx.scene.control.ListView;
import javafx.scene.control.ListView.EditEvent;
import javafx.scene.control.MultipleSelectionModel;
import javafx.scene.control.MultipleSelectionModelBaseShim;
import javafx.scene.control.SelectionMode;
import java.util.List;
import java.util.ArrayList;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

@@ -53,11 +57,23 @@
private ObservableList<String> model;

@Before public void setup() {
Thread.currentThread().setUncaughtExceptionHandler((thread, throwable) -> {
if (throwable instanceof RuntimeException) {
throw (RuntimeException)throwable;
} else {
Thread.currentThread().getThreadGroup().uncaughtException(thread, throwable);
}
});

cell = new ListCell<String>();
model = FXCollections.observableArrayList("Apples", "Oranges", "Pears");
list = new ListView<String>(model);
}

@After public void cleanup() {
Thread.currentThread().setUncaughtExceptionHandler(null);
}

/*********************************************************************
* Tests for the constructors *
********************************************************************/
@@ -833,5 +849,124 @@ public void testListCellHeights() {
cell.maxHeight(-1), 1);
}

@Test
public void testChangeIndexToEditing1_jdk_8264127() {
assertChangeIndexToEditing(0, 1);
}

@Test
public void testChangeIndexToEditing2_jdk_8264127() {
assertChangeIndexToEditing(-1, 1);
}

@Test
public void testChangeIndexToEditing3_jdk_8264127() {
assertChangeIndexToEditing(1, 0);
}

@Test
public void testChangeIndexToEditing4_jdk_8264127() {
assertChangeIndexToEditing(-1, 0);
}



private void assertChangeIndexToEditing(int initialCellIndex, int listEditingIndex) {
list.getFocusModel().focus(-1);
List<EditEvent> events = new ArrayList<EditEvent>();
list.setOnEditStart(e -> {
events.add(e);
});
list.setEditable(true);
cell.updateListView(list);
cell.updateIndex(initialCellIndex);
list.edit(listEditingIndex);
assertEquals("sanity: list editingIndex ", listEditingIndex, list.getEditingIndex());
assertFalse("sanity: cell must not be editing", cell.isEditing());
cell.updateIndex(listEditingIndex);
assertEquals("sanity: index updated ", listEditingIndex, cell.getIndex());
assertEquals("list editingIndex unchanged by cell", listEditingIndex, list.getEditingIndex());
assertTrue(cell.isEditing());
assertEquals(1, events.size());
}

@Test
public void testChangeIndexOffEditing0_jdk_8264127() {
assertUpdateCellIndexOffEditing(1, 0);
}
@Test
public void testChangeIndexOffEditing1_jdk_8264127() {
assertUpdateCellIndexOffEditing(1, -1);
}
@Test
public void testChangeIndexOffEditing2_jdk_8264127() {
assertUpdateCellIndexOffEditing(0, 1);
}
@Test
public void testChangeIndexOffEditing3_jdk_8264127() {
assertUpdateCellIndexOffEditing(0, -1);
}

public void assertUpdateCellIndexOffEditing(int editingIndex, int cancelIndex) {
list.getFocusModel().focus(-1);
List<EditEvent> events = new ArrayList<EditEvent>();
list.setOnEditCancel(e -> {
events.add(e);
});
list.setEditable(true);
cell.updateListView(list);
cell.updateIndex(editingIndex);
list.edit(editingIndex);
assertEquals("sanity: list editingIndex ", editingIndex, list.getEditingIndex());
assertTrue("sanity: cell must be editing", cell.isEditing());
cell.updateIndex(cancelIndex); // change cell index to negative
assertEquals("sanity: index updated ", cancelIndex, cell.getIndex());
assertEquals("list editingIndex unchanged by cell", editingIndex, list.getEditingIndex());
assertFalse("cell must not be editing if cell index is " + cell.getIndex(), cell.isEditing());
assertEquals(1, events.size());
}

@Test
public void testMisbehavingCancelEditTerminatesEdit() {
ListCell<String> cell = new MisbehavingOnCancelListCell<>();

list.setEditable(true);
cell.updateListView(list);

int editingIndex = 1;
int intermediate = 0;
int notEditingIndex = -1;
cell.updateIndex(editingIndex);
list.edit(editingIndex);
assertTrue("sanity: ", cell.isEditing());
try {
list.edit(intermediate);
} catch (Exception ex) {
// just catching to test in finally
} finally {
assertFalse("cell must not be editing", cell.isEditing());
assertEquals("list must be editing at intermediate index", intermediate, list.getEditingIndex());
}
// test editing: second round
// switch cell off editing by cell api
list.edit(editingIndex);
assertTrue("sanity: ", cell.isEditing());
try {
cell.cancelEdit();
} catch (Exception ex) {
// just catching to test in finally
} finally {
assertFalse("cell must not be editing", cell.isEditing());
assertEquals("list editing must be cancelled by cell", notEditingIndex, list.getEditingIndex());
}
}

public static class MisbehavingOnCancelListCell<T> extends ListCell<T> {
@Override
public void cancelEdit() {
super.cancelEdit();
throw new RuntimeException("violating contract");
}
}

}

1 comment on commit 240d28f

@openjdk-notifier

This comment has been minimized.

Copy link

@openjdk-notifier openjdk-notifier bot commented on 240d28f May 24, 2021

Please sign in to comment.