RFR: 8289357: (Tree)TableView is null in (Tree)TableRowSkin during autosize [v5]

Kevin Rushforth kcr at openjdk.org
Wed Jul 13 21:17:07 UTC 2022


On Tue, 5 Jul 2022 23:30:48 GMT, Marius Hanl <mhanl at openjdk.org> wrote:

>> Initialize the `(Tree)TableView` when creating the measure row.
>> This will guarantee, that we can access the `(Tree)TableView` in the `(Tree)TableRowSkin`, which is currently only null during the autosizing (It is always set otherwise).
>> 
>> With this change, a NPE is happening as the `(Tree)TableRow` currently assumes, that there must be a `VirtualFlow` somewhere in the scene (parent). I guard against this now.
>> I remembered, that there is a ticket for the above behaviour here: https://bugs.openjdk.org/browse/JDK-8274065
>> 
>> Finally, the `(Tree)TableRow` must be removed after the autosizing and the index must be set to `-1` (as for the cell) so that e.g. `cancelEdit()` is not triggered. Some tests catched that (see `test_rt_31015`). This did not happen before as the table row setup was not complete, but now the row does not the table and therefore installs some listener on it in order to fire corresponding edit events.
>
> Marius Hanl has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains seven additional commits since the last revision:
> 
>  - Enable tests again
>  - Merge branch 'master' of https://github.com/openjdk/jfx into 8289357-table-view-null-in-table-row-skin
>  - Merge branch 'master' of https://github.com/openjdk/jfx into 8289357-table-view-null-in-table-row-skin
>  - 8289357: Added test to verify, that no (Tree)TableRows remain after auto sizing
>  - 8289357: Fix test which failed as the coutner increased by one due to the now correct row setup
>  - 8289357: Remove (Tree)TableRow after autosizing and update the index to -1 to prevent triggering of listener
>  - 8289357: Initialize the (Tree)TableView when creating the measure row. Also prevent a NPE as we don't have a VirtualFlow in the context of autosizing

The code change looks quite reasonable to me, but it would be good to get additional eyes on this. @aghaisas will also review it.

@kleopatra do you have any thoughts on this?

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

PR: https://git.openjdk.org/jfx/pull/805


More information about the openjfx-dev mailing list