On Fri, 22 Jan 2021 10:08:51 GMT, Ambarish Rapte <ara...@openjdk.org> wrote:
> This particular issue JDK-8256283, is a specific case of IOOBE when, rootItem > is not shown, some children including first child are selected, then all > children are removed and sort() is invoked. The sort() fails with an IOOBE. > This PR only addresses this specific IOOBE. > Root cause of this issue is that the selection is not cleared after rootItems > children are removed. In addition to this, there are few other scenarios when > selection is not updated correctly, which are collected under an umbrella > task [JDK-8248217](https://bugs.openjdk.java.net/browse/JDK-8248217). Fix for > [JDK-8248217](https://bugs.openjdk.java.net/browse/JDK-8248217) would require > good amount refactoring of selection model. > > The fix for this issue is to avoid sort() when > rootItem.getChildren().isEmpty(). > Added a unit test with the fix, which fails without fix and passes with fix. Comments inline. modules/javafx.controls/src/main/java/javafx/scene/control/TreeTableView.java line 582: > 580: try { > 581: TreeItem rootItem = table.getRoot(); > 582: if (rootItem == null | rootItem.getChildren().isEmpty()) > return false; That should be `||` (boolean OR). In addition to being less clear, this will get an NPE if `rootItem` is ever null, since the bit-wise `|` operator doesn't short-curcuit the rest of the `if` test if the first term is true. Given that this would have caused a regression, and that we don't have a test that would catch it, can you add a test that sets the root to null and calls sort? It will fail with this version of the fix, but would pass without the fix and should pass once you update your fix. ------------- Changes requested by kcr (Lead). PR: https://git.openjdk.java.net/jfx/pull/384
