RFR: 8256283: IndexOutOfBoundsException when sorting a TreeTableView [v2]
Ambarish Rapte
arapte at openjdk.java.net
Thu Jan 28 11:52:43 UTC 2021
On Fri, 22 Jan 2021 17:48:15 GMT, Kevin Rushforth <kcr at openjdk.org> wrote:
>> Ambarish Rapte has updated the pull request incrementally with one additional commit since the last revision:
>>
>> Review correction
>
> 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.
Sorry about that, It was a typo. But it did reveal the missing test.
Corrected the operator to `||` and added a test as you suggested which fails only with the commit 1 of this PR.
-------------
PR: https://git.openjdk.java.net/jfx/pull/384
More information about the openjfx-dev
mailing list