[Rev 02] RFR: 8193800: TreeTableView selection changes on sorting

Ambarish Rapte arapte at openjdk.java.net
Wed Jun 3 05:21:09 UTC 2020

> 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:

  kcr review comment changes


  - all: https://git.openjdk.java.net/jfx/pull/244/files
  - new: https://git.openjdk.java.net/jfx/pull/244/files/44052f35..1561d2db

 - full: https://webrevs.openjdk.java.net/jfx/244/webrev.02
 - incr: https://webrevs.openjdk.java.net/jfx/244/webrev.01-02

  Stats: 5 lines in 2 files changed: 0 ins; 1 del; 4 mod
  Patch: https://git.openjdk.java.net/jfx/pull/244.diff
  Fetch: git fetch https://git.openjdk.java.net/jfx pull/244/head:pull/244

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

More information about the openjfx-dev mailing list