On Fri, 26 Jun 2020 09:22:53 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
>> Issue: >> In TreeTableView, in case of Multiple selection mode, if nested items are >> selected, then TreeTableView does not >> retain/update the selection correctly when the tree items are >> permuted(either by `sort()` or by reordering using >> `setAll()`). Cause: >> >> 1. For permutation, the current implementation uses `TreeModificationEvent` >> to update the selection. >> 2. The indices from these TreeModificationEvents are not reliable. >> 3. It uses the non public `TreeTablePosition` constructor to create >> intermediate `TreeItem` positions, this constructor >> results in another unexpected TreeModificationEvent while one for sorting is >> already being processed. 4. In case of >> sorting, there can be multiple intermediate TreeModificationEvents >> generated, and for each TreeModificationEvent, the >> selection gets updated and results in selection change events being >> generated. 5. Each time a TreeItem is expanded or >> collapsed, the selection must be shifted, but shifting is not necessary in >> case of permutation. All these issues >> combine in wrong update of the selection. Fix: >> >> 1. On each TreeModificationEvent for permutation, for updating the >> selection, use index of TreeItem from the >> TreeTableView but not from the TreeModificationEvent. 2. Added a new non >> public TreeTablePosition constructor, which is >> almost a copy constructor but accepts a different row. 3. In case of >> sorting, send out the set of selection change >> events only once after the sorting is over. 4. In case of setAll, send out >> the set of selection change events same as >> before.(setAll results in only one TreeModificationEvent, which effectively >> results in only one set of selection change >> events). `shiftSelection()` should not be called in case of permutation i.e. >> call `if (shift != 0)` >> Verification: >> The change is very limited to updating of selection of TreeTableView items >> when the TreeItems are permuted, so the >> change should not cause any other failures. Added unit tests which fail >> before and pass after the fix. > > Ambarish Rapte has updated the pull request incrementally with one additional > commit since the last revision: > > Correct the bug-id for ignored tests Marked as reviewed by kcr (Lead). ------------- PR: https://git.openjdk.java.net/jfx/pull/244
