RFR: 8251481: TableCell accessing row: NPE on auto-sizing

Marius Hanl mhanl at openjdk.java.net
Thu Jan 27 10:25:39 UTC 2022


On Tue, 25 Jan 2022 12:00:11 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>> This PR will fix a simple NPE which may happens when using the `TableRow` inside a `TableCell` during the initial auto sizing.
>> In the auto sizing code, no `TableRow` is set, therefore `getTableRow()` will return null and it is not possible to e.g. access the row item.
>> 
>> This is fixed by adding the `TableRow` in the `resizeColumnToFitContent` method, similar as it is already done for the `TreeTableView` (`TreeTableRow`).
>
> modules/javafx.controls/src/test/java/test/javafx/scene/control/TableCellTest.java line 371:
> 
>> 369:     @Test
>> 370:     public void testRowIsNotNullWhenAutoSizing() {
>> 371:         TableColumn<String, String> tableColumn = new TableColumn<>();
> 
> - the bug that's fixed in this PR is in TableColumnHeader, shouldn't the test be in TableColumnHeaderTest?
> - if you decide to keep it here: it's in the middle of some edit-related tests, you might consider moving it up/down before/after those
> - the fix aligns the resizeToFit method for TableView with that for TreeTableView: for symmetry, I would also expect a test method for the latter (which will pass both before and after the fix)

I can align it. And yeah makes sense to add a test for the TreeTableView/TreeTableCell.

> modules/javafx.controls/src/test/java/test/javafx/scene/control/TableCellTest.java line 379:
> 
>> 377: 
>> 378:                 assertNotNull(getTableRow());
>> 379:             }
> 
> feeling slightly uncomfortable with the generality of this: looks a bit like it's guaranteed that tableRow != null always (bullet 2 of our previous conversation) - would suggest to make it more specific to auto-sizing (f.i. start without auto-size, then trigger autosizing by code). Or at least doc it (add a message to the assertion, add the bug id) so that future readers will know what exactly is tested here :)

Pretty sure table row is never null. Or is it on some corner case? Because even on an empty table, table rows with empty cells will have the row (since rows will be added in the virtualflow)

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

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


More information about the openjfx-dev mailing list