RFR: 8187145: (Tree)TableView with null selectionModel: throws NPE on sorting [v5]

[Ajit Ghaisas]

On Tue, 23 Aug 2022 20:20:11 GMT, Andy Goryachev <angorya at openjdk.org> wrote:

>> Setting a null selection model in TableView and TreeTableView produce NPE on sorting (and probably in some other situations) because the check for null is missing in several places. 
>> 
>> Setting a null selection model is a valid way to disable selection in a (tree)table.
>> 
>> There is also a similar issue with ListView [JDK-8279640](https://bugs.openjdk.org/browse/JDK-8279640).
>> 
>> changes:
>> - added check for null selection model where it was missing
>> - clear focused row index on setting null selection model in TreeTableView
>> - clear selected cells on setting a new selection model
>
> Andy Goryachev has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 11 commits:
> 
>  - Merge remote-tracking branch 'origin/master' into 8187145.null.selection.model
>  - 8187145: added tests
>  - 8187145: clear selected tree table row when setting null selection model
>  - 8187145: clear selection when setting selection model
>  - Merge remote-tracking branch 'origin/master' into 8187145.null.selection.model
>  - Merge remote-tracking branch 'origin/master' into 8187145.null.selection.model
>  - 8187145: whitespace
>  - 8187145: clear focus
>  - 8187145: tree table view
>  - 8187145: fall through
>  - ... and 1 more: https://git.openjdk.org/jfx/compare/7cb8d679...fed5dfdb

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/CellBehaviorBase.java line 274:

> 272:     protected void simpleSelect(MouseButton button, int clickCount, boolean shortcutDown) {
> 273:         final int index = getIndex();
> 274:         boolean isAlreadySelected;

Code simplification: if `isAlreadySelected` is initialized to `false` on this line, then we can have only `if(sm != null)` condition below instead of if-else.

modules/javafx.controls/src/main/java/com/sun/javafx/scene/control/behavior/TableCellBehaviorBase.java line 203:

> 201:     protected void simpleSelect(MouseButton button, int clickCount, boolean shortcutDown) {
> 202:         final TableSelectionModel<S> sm = getSelectionModel();
> 203:         boolean isAlreadySelected;

Code simplification: if `isAlreadySelected` is initialized to `false` on this line, then we can have only `if(sm != null)` condition below instead of if-else.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java line 178:

> 176:                 return null;
> 177:             }
> 178:             // fall through

There is no "fall through" here as we return from all conditions in previous case.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TableRowSkin.java line 192:

> 190:                 return null;
> 191:             }
> 192:             // fall through

There is no "fall through" here as we return from all conditions in previous case.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java line 486:

> 484:                 return null;
> 485:             }
> 486:             // fall through

There is no "fall through" here as we return from all conditions in previous case.

modules/javafx.controls/src/main/java/javafx/scene/control/skin/TreeTableRowSkin.java line 500:

> 498:                 return null;
> 499:             }
> 500:             // fall through

There is no "fall through" here as we return from all conditions in previous case.

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

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