On Tue, 5 Jul 2022 23:30:48 GMT, Marius Hanl <mh...@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
