RFR: 8196065: ListChangeListener getRemoved() returns items that were not removed. [v6]

[Jeanette Winzenburg]

On Mon, 17 May 2021 16:16:16 GMT, Michael Strauß <mstrauss at openjdk.org> wrote:

>> The documentation for `ObservableListBase.nextRemove` states that a single change always refers to the current state of the list, which likely means that multiple disjoint removed ranges need to be applied in order, otherwise the next change's `getFrom` doesn't refer to the correct index.
>> 
>> `SelectedItemsReadOnlyObservableList` doesn't apply removals to `itemsRefList`, which means that subsequent removals will refer to the wrong index when retrieving the removed elements. This PR fixes the calculation of the current index.
>
> Michael Strauß has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 10 commits:
> 
>  - Merge branch 'master' into fixes/JDK-8196065
>    
>    # Conflicts:
>    #	modules/javafx.controls/src/test/java/test/javafx/scene/control/MultipleSelectionModelImplTest.java
>  - Merge branch 'master' into fixes/JDK-8196065
>  - Merge branch 'master' into fixes/JDK-8196065
>    
>    # Conflicts:
>    #	modules/javafx.controls/src/main/java/javafx/scene/control/ControlUtils.java
>  - Cleanup
>  - Fixed clear-and-select change notification
>  - Add failing tests
>  - Cleanup
>  - Added tests
>  - Fix incorrect index when multiple remove changes are handled in SelectedItemsReadOnlyObservableList
>  - Add failing test

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/SelectedItemsReadOnlyObservableList.java line 54:

> 52:             while (c.next()) {
> 53:                 if (c.wasReplaced()) {
> 54:                     List<E> removed = getRemovedElements(c, totalRemovedSize);

hmm, don't quite understand all the fuss (but might be overlooking something, has been a while since I did a deeper dig into the selection issues ;) 

- we have a copy of the items (itemsRefs) with the same coordinates we receive from the change
- this list is in sync with the indices, that is it contains the items at the respective selectedIndices
- currently, that's done in a bulk setting at the end of the listener code

Why not sync our copy while working through the change? Doing so, would automatically collect the state of our own change, or what am I missing?

-------------

PR: https://git.openjdk.java.net/jfx/pull/478