On Mon, 29 Jul 2024 22:00:39 GMT, Andy Goryachev <ango...@openjdk.org> wrote:
>> This PR fixes a bug where a `TableView` inside a `TitledPane` may
>> 'duplicate' the cells. This actually has nothing to do with the
>> `TitledPane`, but it triggered the bug easily due to its nature to change
>> the size of his content when collapsing.
>>
>> The expansion change of a `TitledPane` triggered an event where the
>> underlying `VirtualFlow` was adding cells to the pile (for reuse) and later
>> cleaning them all up without resetting the index to -1.
>> This led to a bug where two cells had the same index and therefore received
>> an edit event, although just one should (and is visible, the other cell has
>> no parent -> orphan node).
>>
>> The fix is to always reset the index to -1. This was already done before,
>> just not everywhere, for all cells.
>> So before we clear the pile (or cells), we always reset the index to -1.
>> Added a bunch of tests, some pass before and after.
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/VirtualFlow.java
> line 808:
>
>> 806: vertical = new BooleanPropertyBase(true) {
>> 807: @Override protected void invalidated() {
>> 808: resetIndex(cells);
>
> minor: [cells|pile]clear() can be combined with resetIndex(), something like
>
>
> private void clear(ArrayLinkedList<T> cells) {
> for (T cell : cells) {
> cell.updateIndex(-1);
> }
> cells.clear();
> }
>
>
> as these operations seem to be called in pairs
Unfortunately not in the `needsRebuildCells` if condition, where we only reset
the index and add all cells to the pile
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/1521#discussion_r1696422974
