On Tue, 23 Aug 2022 20:20:11 GMT, Andy Goryachev <[email protected]> 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
