[prev in list] [next in list] [prev in thread] [next in thread] 

List:       openjdk-openjfx-dev
Subject:    Re: RFR: 8256397: MultipleSelectionModel throws IndexOutOfBoundException
From:       Jeanette Winzenburg <fastegal () openjdk ! java ! net>
Date:       2020-11-17 12:41:00
Message-ID: j8Su5tN_UxIKSt1v8UsgmXc7wlsLEfOGf-HHiUHe4d4=.538ab502-4359-4b77-832b-ff3cf78ef800 () github ! com
[Download RAW message or body]

On Mon, 16 Nov 2020 14:41:57 GMT, Florian Kirmaier <fkirmaier@openjdk.org> wrote:

> Fixing IndexOutOfBoundsException in the MultipleSelectionModelBase and added a \
>                 unit-test for it.
> ticket: https://bugs.openjdk.java.net/browse/JDK-8256397
> run test: `./gradlew --continue -PFULL_TEST=true controls:test --tests \
> "*MultipleSelectionModelImplTest*"`

good catch :)

No review, just a couple of comments:

- other subclasses of MultipleSelectionModelBase might have similar issues 
- the fix gets rid of the error (good!) but might fire incorrect changes (see below)
- there are quite a lot of open issues related to change notification, some might \
                profit from a fix of this
- tests, tests, tests .. :)

selectIndices might involve disjoint intervals in the change (even for addition which \
is unusual in the "normal" implementations of observableLists)

       // initial selection, starting from empty
       selectionModel.selectIndices(0, /*1, IMPORTANT: remove some index */ 2,  3);
       System.out.println(change);
      // expected and actual change 
      { [0, 2, 3] added at 0 }

      // add selection of disjoint indices (assuming a longer items list)
       selectionModel.selectIndices(1, 5, 7);
       System.out.println(change);
      // actual
       { [1, 2, 3] added at 1 }
      // expected
     { [1] added at 1, [5, 7] added at 4 }

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

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


[prev in list] [next in list] [prev in thread] [next in thread]