RFR: 8289357: (Tree)TableView is null in (Tree)TableRowSkin during autosize [v5]
Kevin Rushforth
kcr at openjdk.org
Tue Aug 9 21:38:40 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 know 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
I just noticed comments in [JDK-8292009](https://bugs.openjdk.org/browse/JDK-8292009) indicating that this PR fixes that bug as well. It would be good to get the review for this fix done.
Since JDK-8292009 described a different problem that is also fixed as a result, it seems good to add that issue to this PR.
@Maran23 GitHub is reporting a merge conflict in two of the test files. Can you merge the latest upstream master into your branch?
-------------
PR: https://git.openjdk.org/jfx/pull/805
More information about the openjfx-dev
mailing list