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

[Kevin Rushforth]

On Thu, 8 Dec 2022 16:16:58 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 21 commits:
> 
>  - Merge remote-tracking branch 'origin/master' into 8187145.null.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: cleanup
>  - 8187145: also tree table view
>  - 8187145: whitespace
>  - Merge remote-tracking branch 'origin/master' into 8187145.null.selection.model
>  - 8187145: review comments
>  - 8187145: github
>  - Merge remote-tracking branch 'origin/master' into 8187145.null.selection.model
>  - ... and 11 more: https://git.openjdk.org/jfx/compare/6abbe080...5093f056

Looks good. I noted two minor formatting issues. If you fix them, I'll reapprove.

modules/javafx.controls/src/main/java/javafx/scene/control/TableView.java line 1608:

> 1606: 
> 1607:                 List<TablePosition<S, ?>> removed = new ArrayList<>();
> 1608:                 if(prevState != null) {

Minor: missing space after the `if`.

modules/javafx.controls/src/test/java/test/javafx/scene/control/TreeAndTableViewTest.java line 230:

> 228:     protected static boolean containsPseudoclass(Node n, String pseudoclass) {
> 229:         for (PseudoClass pc: n.getPseudoClassStates()) {
> 230:             if(pseudoclass.equals(pc.getPseudoClassName())) {

Minor: missing space after `if`

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

Marked as reviewed by kcr (Lead).

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