On Fri, 17 Apr 2026 22:17:50 GMT, Andy Goryachev <[email protected]> wrote:
>> This fixes an IOOBE and even improves the performance / change aggregation.
>> I suggest reading Jeanette's comment in the ticket, that helped a lot to
>> understand it.
>>
>> There was a code path that broke change events.
>> Jeanette wondered as well in the comment why this exists at all, if this was
>> meant to be some kind of optimization?
>>
>> What I found out is: Because of this code path, we actually got 3 change
>> events and the order was wrong.
>> What we got with the test from Jeanette: `subChanges: { [B] removed at 1,
>> [D] removed at 2, [C, E, F] removed at 1 }`
>>
>> Removing the code path and just calling `insertRemoved(..)` directly will
>> correctly find an alreadx existing sub change with `findSubChange` and
>> aggregate the changes together. So now, we only get one event: `subChanges:
>> { [B, C, D, E, F] removed at 1 }`. No error and we use the smart aggregation
>> from `insertRemoved`.
>>
>> My speculation why this code existed is that there may was a former bug,
>> similar like [JDK-8208758](https://bugs.openjdk.org/browse/JDK-8208758)
>> (#2117) where I also did a fix in the `insertRemoved(..)` method.
>>
>> ---------
>> - [x] I confirm that I make this contribution in accordance with the
>> [OpenJDK Interim AI Policy](https://openjdk.org/legal/ai).
>
> modules/javafx.base/src/test/java/test/javafx/collections/ListChangeBuilderTest.java
> line 744:
>
>> 742: list.remove("C");
>> 743:
>> 744: list.doEndChange();
>
> do you think it would make sense to add scenarios with .filtered() and
> .sorted() described in the reproducer for
> https://bugs.openjdk.org/browse/JDK-8237868 as tests?
>
> https://bugs.openjdk.org/secure/attachment/86526/SampleApp.java
While it is not a problem inside `FilteredList` and `SortedList`, more tests
don't hurt. So I will have a look. Especially since we don't have many tests
for the `FilteredList` and `SortedList`.
-------------
PR Review Comment: https://git.openjdk.org/jfx/pull/2154#discussion_r3103616816
