Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
8165214: ListView.EditEvent.getIndex() does not return the correct index 
Reviewed-by: jvos, aghaisas
  • Loading branch information
Jeanette Winzenburg committed Jun 23, 2021
1 parent 13cffba commit 04493e5
Show file tree
Hide file tree
Showing 2 changed files with 101 additions and 7 deletions.
14 changes: 8 additions & 6 deletions modules/javafx.controls/src/main/java/javafx/scene/control/ListCell.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2010, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2010, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand Down Expand Up @@ -352,6 +352,8 @@ public void changed(
* Editing API *
* *
**************************************************************************/
// index at time of startEdit - fix for JDK-8165214
private int indexAtStartEdit;

/** {@inheritDoc} */
@Override public void startEdit() {
Expand All @@ -374,6 +376,8 @@ public void changed(
list.edit(getIndex());
list.requestFocus();
}

indexAtStartEdit = getIndex();
}

/** {@inheritDoc} */
Expand Down Expand Up @@ -418,13 +422,11 @@ public void changed(
@Override public void cancelEdit() {
if (! isEditing()) return;

// Inform the ListView of the edit being cancelled.
ListView<T> list = getListView();

super.cancelEdit();

// Inform the ListView of the edit being cancelled.
ListView<T> list = getListView();
if (list != null) {
int editingIndex = list.getEditingIndex();

// reset the editing index on the ListView
if (updateEditingIndex) list.edit(-1);
Expand All @@ -438,7 +440,7 @@ public void changed(
list.fireEvent(new ListView.EditEvent<T>(list,
ListView.<T>editCancelEvent(),
null,
editingIndex));
indexAtStartEdit));
}
}

Expand Down
94 changes: 93 additions & 1 deletion modules/javafx.controls/src/test/java/test/javafx/scene/control/ListCellTest.java
View file
@@ -1,5 +1,5 @@
/*
* Copyright (c) 2011, 2020, Oracle and/or its affiliates. All rights reserved.
* Copyright (c) 2011, 2021, Oracle and/or its affiliates. All rights reserved.
* DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER.
*
* This code is free software; you can redistribute it and/or modify it
Expand All @@ -26,6 +26,7 @@
package test.javafx.scene.control;

import javafx.scene.control.skin.ListCellSkin;
import test.com.sun.javafx.scene.control.infrastructure.StageLoader;
import javafx.beans.InvalidationListener;
import javafx.collections.FXCollections;
import javafx.collections.ListChangeListener;
Expand All @@ -39,12 +40,15 @@
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;

import com.sun.javafx.tk.Toolkit;

import static javafx.scene.control.ControlShim.*;
import static test.com.sun.javafx.scene.control.infrastructure.ControlTestUtils.*;
import static org.junit.Assert.*;
Expand All @@ -55,6 +59,7 @@ public class ListCellTest {
private ListCell<String> cell;
private ListView<String> list;
private ObservableList<String> model;
private StageLoader stageLoader;

@Before public void setup() {
Thread.currentThread().setUncaughtExceptionHandler((thread, throwable) -> {
Expand All @@ -71,6 +76,7 @@ public class ListCellTest {
}

@After public void cleanup() {
if (stageLoader != null) stageLoader.dispose();
Thread.currentThread().setUncaughtExceptionHandler(null);
}

Expand Down Expand Up @@ -750,6 +756,92 @@ protected void assertInRangeState(int index) {
assertFalse(cell.isEditing());
}

@Test
public void testEditCancelEventAfterCancelOnCell() {
list.setEditable(true);
cell.updateListView(list);
int editingIndex = 1;
cell.updateIndex(editingIndex);
list.edit(editingIndex);
List<EditEvent<String>> events = new ArrayList<>();
list.setOnEditCancel(events::add);
cell.cancelEdit();
assertEquals(1, events.size());
assertEquals("editing location of cancel event", editingIndex, events.get(0).getIndex());
}

@Test
public void testEditCancelEventAfterCancelOnList() {
list.setEditable(true);
cell.updateListView(list);
int editingIndex = 1;
cell.updateIndex(editingIndex);
list.edit(editingIndex);
List<EditEvent<String>> events = new ArrayList<>();
list.setOnEditCancel(events::add);
list.edit(-1);
assertEquals(1, events.size());
assertEquals("editing location of cancel event", editingIndex, events.get(0).getIndex());
}

@Test
public void testEditCancelEventAfterChangeEditingIndexOnList() {
list.setEditable(true);
cell.updateListView(list);
int editingIndex = 1;
cell.updateIndex(editingIndex);
list.edit(editingIndex);
List<EditEvent<String>> events = new ArrayList<>();
list.setOnEditCancel(events::add);
list.edit(0);
assertEquals(1, events.size());
assertEquals("editing location of cancel event", editingIndex, events.get(0).getIndex());
}

@Test
public void testEditCancelEventAfterCellReuse() {
list.setEditable(true);
cell.updateListView(list);
int editingIndex = 1;
cell.updateIndex(editingIndex);
list.edit(editingIndex);
List<EditEvent<String>> events = new ArrayList<>();
list.setOnEditCancel(events::add);
cell.updateIndex(0);
assertEquals(1, events.size());
assertEquals("editing location of cancel event", editingIndex, events.get(0).getIndex());
}

@Test
public void testEditCancelEventAfterModifyItems() {
list.setEditable(true);
stageLoader = new StageLoader(list);
int editingIndex = 1;
list.edit(editingIndex);
List<EditEvent<String>> events = new ArrayList<>();
list.setOnEditCancel(events::add);
list.getItems().add(0, "added");
Toolkit.getToolkit().firePulse();
assertEquals(1, events.size());
assertEquals("editing location of cancel event", editingIndex, events.get(0).getIndex());
}

@Test
public void testEditCancelEventAfterRemoveEditingItem() {
list.setEditable(true);
stageLoader = new StageLoader(list);
int editingIndex = 1;
list.edit(editingIndex);
List<EditEvent<String>> events = new ArrayList<>();
list.setOnEditCancel(events::add);
list.getItems().remove(editingIndex);
Toolkit.getToolkit().firePulse();
assertEquals("removing item must cancel edit on list", -1, list.getEditingIndex());
assertEquals(1, events.size());
assertEquals("editing location of cancel event", editingIndex, events.get(0).getIndex());
}


// 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?

Expand Down

1 comment on commit 04493e5

@openjdk-notifier
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review

Issues

Please sign in to comment.