Re: [Rev 01] RFR: 8242489: ChoiceBox: initially toggle not sync'ed to selection

2020-04-16 Thread Jeanette Winzenburg
On Thu, 16 Apr 2020 11:23:24 GMT, Ambarish Rapte wrote: >>> 1. do nothing for a (don't feel like filing yet another bug around >>> selection ;) and b (the skin behaves correctly, I >>> think) >> >> I am good with this. Though I will file a JBS for the correction in >> ChoiceBoxSelectio

Re: [Rev 01] RFR: 8242489: ChoiceBox: initially toggle not sync'ed to selection

2020-04-16 Thread Ambarish Rapte
On Thu, 16 Apr 2020 11:18:55 GMT, Ambarish Rapte wrote: >> btw: just noticed that there are methods in ChoiceBoxSkin testing the fix >> for next/prev >> >> @Test public void test_jdk_8988261_selectNext() { >> @Test public void test_jdk_8988261_selectPrevious() { >> >> the name look lik

Re: [Rev 01] RFR: 8242489: ChoiceBox: initially toggle not sync'ed to selection

2020-04-16 Thread Ambarish Rapte
On Thu, 16 Apr 2020 09:17:19 GMT, Jeanette Winzenburg wrote: >> yeah, you are right: >> >> a) the implementation of ChoiceBoxSelectionModel is broken when it comes to >> handling of unselectable items (such as >> Separator): next/previous try to move on, the others simply select. The >> imple

Re: [Rev 01] RFR: 8242489: ChoiceBox: initially toggle not sync'ed to selection

2020-04-16 Thread Jeanette Winzenburg
On Thu, 16 Apr 2020 09:05:12 GMT, Jeanette Winzenburg wrote: >> modules/javafx.controls/src/main/java/javafx/scene/control/skin/ChoiceBoxSkin.java >> line 416: >> >>> 415: } else { >>> 416: toggleGroup.selectToggle(null); >>> 417: } >> >> Th

Re: [Rev 01] RFR: 8242489: ChoiceBox: initially toggle not sync'ed to selection

2020-04-16 Thread Jeanette Winzenburg
On Wed, 15 Apr 2020 15:52:13 GMT, Ambarish Rapte wrote: >> Jeanette Winzenburg has updated the pull request incrementally with one >> additional commit since the last revision: >> >> ChoiceBox: added FIXME with reference to issue > > modules/javafx.controls/src/main/java/javafx/scene/control/

Re: [Rev 01] RFR: 8242489: ChoiceBox: initially toggle not sync'ed to selection

2020-04-16 Thread Jeanette Winzenburg
On Wed, 15 Apr 2020 15:46:16 GMT, Ambarish Rapte wrote: >> Jeanette Winzenburg has updated the pull request incrementally with one >> additional commit since the last revision: >> >> ChoiceBox: added FIXME with reference to issue > > modules/javafx.controls/src/main/java/javafx/scene/control/

Re: [Rev 01] RFR: 8242489: ChoiceBox: initially toggle not sync'ed to selection

2020-04-15 Thread Ambarish Rapte
On Wed, 15 Apr 2020 10:02:47 GMT, Jeanette Winzenburg wrote: >> Macroscopic issue is that initially, the toggle is not sync'ed to the >> selection state. Root reason is an missing else >> block when updating toggle selection state (see report for details). >> Fixed by introducing the else block

Re: [Rev 01] RFR: 8242489: ChoiceBox: initially toggle not sync'ed to selection

2020-04-15 Thread Ajit Ghaisas
On Wed, 15 Apr 2020 10:02:47 GMT, Jeanette Winzenburg wrote: >> Macroscopic issue is that initially, the toggle is not sync'ed to the >> selection state. Root reason is an missing else >> block when updating toggle selection state (see report for details). >> Fixed by introducing the else block

Re: [Rev 01] RFR: 8242489: ChoiceBox: initially toggle not sync'ed to selection

2020-04-15 Thread Jeanette Winzenburg
On Tue, 14 Apr 2020 14:09:13 GMT, Ajit Ghaisas wrote: >> Jeanette Winzenburg has updated the pull request incrementally with one >> additional commit since the last revision: >> >> ChoiceBox: added FIXME with reference to issue > > modules/javafx.controls/src/main/java/javafx/scene/control/Ch

Re: [Rev 01] RFR: 8242489: ChoiceBox: initially toggle not sync'ed to selection

2020-04-15 Thread Jeanette Winzenburg
> Macroscopic issue is that initially, the toggle is not sync'ed to the > selection state. Root reason is an missing else > block when updating toggle selection state (see report for details). > Fixed by introducing the else block and removing all follow-up errors that > tried to amend the conseq