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

Jeanette Winzenburg fastegal at openjdk.java.net
Thu Jun 4 08:40:25 UTC 2020


On Wed, 3 Jun 2020 13:45:12 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:

>>> The algorithm looks correct to me for sorting. How much regression testing have you done for cases where rows or
>>> columns are inserted?
>> 
>> I have tested with a small TreeTableView of 15 rows of 3 levels and 3 columns.
>> Would it be enough to test with TreeTableView of ~500 rows of 5 levels, 10 columns ?
>> 
>> Update:  I am testing with 7 level of nested rows, with 10, 9, 7, 6, 5, 4, 3 number of children in each level
>> respectively. The fix works fine till level 3. But can observe issue with level 4 and further. Shall debug this more
>> and update.
>
>> I have tested with a small TreeTableView of 15 rows of 3 levels and 3 columns.
>> Would it be enough to test with TreeTableView of ~500 rows of 5 levels, 10 columns ?
>> 
>> Update: I am testing with 7 level of nested rows, with 10, 9, 7, 6, 5, 4, 3 number of children in each level
>> respectively. The fix works fine till level 3. But can observe issue with level 4 and further. Shall debug this more
>> and update.
> 
> OK, thanks. In addition to making sure that insertion, deletion, and sorting work, are you also checking that the
> events being sent are as expected?

Unrelated to this fix I see two pain points (which were not introduced here :)

#### A. replaced vs. permutated

These are types of change as decided by the list (aka: model) - treeModificationEvent (aka: view) must follow the lead
(vs. spreading its own interpretation). Doing so is a bug, IMO.

#### B. treeTableView.sort changing internals of the selectionModel

that's a no-no-no-go, always. Instead let the selectionModel handle the permutated state correctly (didn't dig why it
was deemed necessary to do so in sort - though it has the side-effect of leaving out selectionModels that are not of
the expected type)

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

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


More information about the openjfx-dev mailing list