On Sun, 25 Apr 2021 15:36:49 GMT, mstr2 <github.com+43553916+ms...@openjdk.org> wrote:
> > > You're right that removing `ControlUtils.reducingChange` is enough to solve > the problem. Maybe the method was created because it might be confusing that, > while the indices are often not adjacent, they _are_ (as you correctly say) > adjacent with respect to `selectedIndices`. > > I have tested the change with > [8189354](https://bugs.openjdk.java.net/browse/JDK-8189354) and > [8189228](https://bugs.openjdk.java.net/browse/JDK-8189228), and both issues > seem to be solved. yeah, coordinate spaces can be confusing, been there :) A couple of notes to your test - my personal preferences: - they should cover all changes, that is test both Tree- and TreeTableView - if possible, have a test that really cover the report: here that would be throwing a AIOOP on expanding again (with a filter to re-select all children) - have a test for underlying reason of the reported misbehavior: here the incorrect change - testing the change should be complete (in testing its state) I have seen you using the MockListObserver in an earlier version of this PR - which is perfect for the last bullet, we should use it always in asserting the correctness of listChange notification :) The only very minor drawback is that it requires to update the Eclipse classpath to add-exports for the base testing package (not a big deal). ------------- PR: https://git.openjdk.java.net/jfx/pull/480
