RFR: 8193800: TreeTableView selection changes on sorting [v5]

Ambarish Rapte arapte at openjdk.java.net
Wed Jun 24 10:23:25 UTC 2020

On Tue, 23 Jun 2020 13:09:44 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

> While digging a bit (mainly around my [earlier
> concerns](https://github.com/openjdk/jfx/pull/244#issuecomment-638702570) - which still hold but could be discussed in
> a follow-up issue :)

Yes, We need to address these issues. How should we treat permutation vs replace, which may involve answering few more
small questions. Like, What should be the selected item when we remove the current selected item? What should the
contents of an event be in such cases?

Regarding the scenario in your test, In `TreeTableView`, when `setAll()` is called by providing the exact same items in
a different order, then it is considered as permutation. and a `wasPermutated` `TreeModificationEvent` gets generated.
But if even a single `TreeItem` is removed and `setAll(` with the remaining n-1 TreeItems`)` is called, then it is
considered as adding new items. (May be it should be generate as `wasRemoved`). And similar issue can be observed when
a `TreeItem` is added.

In the test that you provided, one `TreeItem` is removed before doing `setAll()`. So this results in a `wasAdded`
`TreeModificationEvent `and does not enter the `wasPermutated` code block. This fix only changes the `wasPermutated`
event. So the behavior is still buggy more or less similar to before this change. There are multiple such issues with
updating the selection. I have grouped few already reported issues under

> I came up with a NPE thrown by the newly added code block ins sort (the one walking up the parent hierarchy).

I have added a null check for now, with a reasoning comment. We will have to remove that check once we fix the other
issues. Thanks for the providing the test :). code really helps to understand. I have added this scenario to test along
with few others. The tests are ignored using [JDK-8248217](https://bugs.openjdk.java.net/browse/JDK-8248217). Please
take a look.


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

More information about the openjfx-dev mailing list