RFR: 8258663: Fixed size TableCells are not removed from sene graph when column is removed

[Marius Hanl]

On Mon, 29 Mar 2021 11:40:40 GMT, Jeanette Winzenburg <fastegal at openjdk.org> wrote:

>> This PR fixes an issue, where table cells are not removed from the table row when the corresponding table column got removed. This will lead to empty "ghost" cells laying around in the table.
>> This bug only occurs, when a fixed cell size is set to the table.
>> 
>> I also added 3 more tests beside the mandatory test, which tests my fix.
>> 1 alternative test, which tests the same with no fixed cell size set.
>> The other 2 tests are testing the same, but the table columns are set invisible instead.
>> 
>> -> Either the removal or setVisible(false) of a column should both do the same: Remove the corresponding cells, so that there are no empty cells.
>> Therefore, the additional tests make sure, that the other use cases (still) works and won't break in future (at least, without red tests ;)).
>> See also: TableRowSkinBase#updateCells(boolean)
>
> modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkinBase.java line 556:
> 
>> 554:             }
>> 555:             getChildren().removeAll(toRemove);
>> 556:         } else if (resetChildren || cellsEmpty) {
> 
> just curious : any idea why fixedCellSize was special-cased here? Not to clean them up sounds (and is) very much wrong, so there must have been a reason?

The '!fixedCellSizeEnabled' check is not needed, as we already check for fixedCellSizeEnabled in the main if condition.
`if (fixedCellSizeEnabled) {...}`

So before, it was like: 
`if (fixedCellSizeEnabled) {...} `
`else if (!fixedCellSizeEnabled && (resetChildren || cellsEmpty)) {...}` 

So we only execute the else condition, if fixedCellSizeEnabled is not true -> !fixedCellSizeEnabled. -> The check is not necessary, as fixedCellSizeEnabled must be false, when we come to the else condition.
The variable fixedCellSizeEnabled is also a primitive boolean, so it can't be null, making this check obsolete.

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

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