Re: [Integrated] RFR: 8241999: ChoiceBox: incorrect toggle selected for uncontained
On Tue, 28 Apr 2020 15:36:53 GMT, Jeanette Winzenburg wrote: > The issue is that the toggles is not reliably unselected if an uncontained > value is set. > > The root is ChoiceBoxSelectionModel which doesn't update the index on > selecting an uncontained item, in particular it > fails to keep the invariant: > assertEquals(getItems().indexOf(selectedItem), selectedIndex); > > The fix here is to override select(item) to guarantee the assert. > > Added/removed ignore from tests that failed before and pass after the fix. > All other tests are passing before and after. This pull request has now been integrated. Changeset: 99f77475 Author:Jeanette Winzenburg Committer: Ajit Ghaisas URL: https://git.openjdk.java.net/jfx/commit/99f77475 Stats: 55 lines in 2 files changed: 7 ins; 44 del; 4 mod 8241999: ChoiceBox: incorrect toggle selected for uncontained Reviewed-by: aghaisas - PR: https://git.openjdk.java.net/jfx/pull/200
Re: RFR: 8241999: ChoiceBox: incorrect toggle selected for uncontained
On Tue, 28 Apr 2020 15:36:53 GMT, Jeanette Winzenburg wrote: > The issue is that the toggles is not reliably unselected if an uncontained > value is set. > > The root is ChoiceBoxSelectionModel which doesn't update the index on > selecting an uncontained item, in particular it > fails to keep the invariant: > assertEquals(getItems().indexOf(selectedItem), selectedIndex); > > The fix here is to override select(item) to guarantee the assert. > > Added/removed ignore from tests that failed before and pass after the fix. > All other tests are passing before and after. Marked as reviewed by aghaisas (Reviewer). - PR: https://git.openjdk.java.net/jfx/pull/200
Re: RFR: 8241999: ChoiceBox: incorrect toggle selected for uncontained
On Tue, 28 Apr 2020 20:44:01 GMT, Kevin Rushforth wrote: >> The issue is that the toggles is not reliably unselected if an uncontained >> value is set. >> >> The root is ChoiceBoxSelectionModel which doesn't update the index on >> selecting an uncontained item, in particular it >> fails to keep the invariant: >> assertEquals(getItems().indexOf(selectedItem), selectedIndex); >> >> The fix here is to override select(item) to guarantee the assert. >> >> Added/removed ignore from tests that failed before and pass after the fix. >> All other tests are passing before and after. > > @aghaisas can you review this? It looks pretty simple so one reviewer should > be enough, unless you spot something where > you want a second pair of eyes. @kevinrushforth, I agree on single reviewer being enough for this change. The fix looks good to me. - PR: https://git.openjdk.java.net/jfx/pull/200
Re: RFR: 8241999: ChoiceBox: incorrect toggle selected for uncontained
On Tue, 28 Apr 2020 15:36:53 GMT, Jeanette Winzenburg wrote: > The issue is that the toggles is not reliably unselected if an uncontained > value is set. > > The root is ChoiceBoxSelectionModel which doesn't update the index on > selecting an uncontained item, in particular it > fails to keep the invariant: > assertEquals(getItems().indexOf(selectedItem), selectedIndex); > > The fix here is to override select(item) to guarantee the assert. > > Added/removed ignore from tests that failed before and pass after the fix. > All other tests are passing before and after. @aghaisas can you review this? It looks pretty simple so one reviewer should be enough, unless you spot something where you want a second pair of eyes. - PR: https://git.openjdk.java.net/jfx/pull/200
RFR: 8241999: ChoiceBox: incorrect toggle selected for uncontained
The issue is that the toggles is not reliably unselected if an uncontained value is set. The root is ChoiceBoxSelectionModel which doesn't update the index on selecting an uncontained item, in particular it fails to keep the invariant: assertEquals(getItems().indexOf(selectedItem), selectedIndex); The fix here is to override select(item) to guarantee the assert. Added/removed ignore from tests that failed before and pass after the fix. All other tests are passing before and after. - Commit messages: - 8241999: ChoiceBox: incorrect toggle selected for uncontained Changes: https://git.openjdk.java.net/jfx/pull/200/files Webrev: https://webrevs.openjdk.java.net/jfx/200/webrev.00 Issue: https://bugs.openjdk.java.net/browse/JDK-8241999 Stats: 55 lines in 2 files changed: 44 ins; 7 del; 4 mod Patch: https://git.openjdk.java.net/jfx/pull/200.diff Fetch: git fetch https://git.openjdk.java.net/jfx pull/200/head:pull/200 PR: https://git.openjdk.java.net/jfx/pull/200
