On Tue, 25 Jan 2022 12:00:11 GMT, Jeanette Winzenburg <faste...@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

Reply via email to