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

Kevin Rushforth kcr at openjdk.org
Tue Dec 6 23:02:19 UTC 2022


On Wed, 10 Aug 2022 08:14:13 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 pull request now contains ten commits:
> 
>  - Added other ticket as reference in javadoc
>  - Merge branch 'master' of https://github.com/openjdk/jfx into 8289357-table-view-null-in-table-row-skin
>    
>     Conflicts:
>    	modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TableRowSkinTest.java
>    	modules/javafx.controls/src/test/java/test/javafx/scene/control/skin/TreeTableRowSkinTest.java
>  - 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

Given how far behind your branch is from master, I agree with Andy that it would be helpful to merge the latest master so that we could see a GHA run and ensure no strange interactions.

I'll approve it as is, though, since I did do a quick local test (on Windows only), and I always do such tests on a branch where I merge in the latest master.

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

Marked as reviewed by kcr (Lead).

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


More information about the openjfx-dev mailing list