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/test/java/test/com/sun/javafx/scene/control/SelectedItemsReadOnlyObservableListTest.java line 120:

> 118:         assertEquals(change(replaced(0, range("foo"), range("bar"))), changes.get(0));
> 119:     }
> 120: 

Another thingy I don't quite understand, sry ;) 

What do you want to test here? We have:

selectedItems before: [foo, bar]
selectedItems after: [bar, foo]

so we need a change (the "should be" snippet in the description of this method)

replaced [foo, bar] by [bar, foo] at 0  

we get: 

replaced [foo] by [bar] at 0

which is an incorrect notification (because it's incomplete, doesn't notify at all about the replace at 1) covered by a passing test .. feels fishy. The other way round: if selectedItems cannot react to such a use-case with a correct notification, it might be broken?

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

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