On Thu, 8 Dec 2022 16:16:58 GMT, Andy Goryachev <ango...@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
