On Thu, 22 Apr 2021 11:44:41 GMT, Jeanette Winzenburg <faste...@openjdk.org>
wrote:
>> Florian Kirmaier has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> 8264127:
>> Fixed another index case based on code review
>
> modules/javafx.controls/src/main/java/javafx/scene/control/ListCell.java line
> 549:
>
>> 547: cancelEdit();
>> 548: }
>> 549: } else {
>
> minor: not a big fan of local fields, but .. if there already _is_ a local
> field, we should use it instead of calling the method again (here: editing
> vs. isEditing)
>
> minor: the code comment does no longer fit the code
>
> major: this will change the editing state of the list - the call to
> cancelEdit must be wrapped into un/setting the updateEditingIndex flag (see
> the else-if block later in the method)
done
> modules/javafx.controls/src/test/java/test/javafx/scene/control/ListCellTest.java
> line 845:
>
>> 843: events.add(e);
>> 844: });
>> 845: list.setEditable(true);
>
> missing setup of focus to -1
I've removed this test in favor of the test-method below.
> modules/javafx.controls/src/test/java/test/javafx/scene/control/ListCellTest.java
> line 911:
>
>> 909: assertTrue("sanity: cell must be editing", cell.isEditing());
>> 910: cell.updateIndex(-1); // change cell index to negative
>> 911: assertFalse("cell must not be editing if cell index is " +
>> cell.getIndex(), cell.isEditing());
>
> missing: assert that a cancelEvent is fired and the list editing state is
> unchanged (they fail until updateEditing is fixed)
The check of cancel-events is now added.
-------------
PR: https://git.openjdk.java.net/jfx/pull/441
