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

[Jeanette Winzenburg]

On Thu, 16 Apr 2020 11:23:24 GMT, Ambarish Rapte <arapte at openjdk.org> 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 ChoiceBoxSelectionModel.
>> `seletPrevious()`, `selectNext()` need one more check `value instanceof SeparatorMenuItem`.
>> and similarly `selectFirst()` and `selectLast()` should be overridden correctly.
>> and I can't think of why `select()` was changed so may be rethink about it :).
>> We can discuss it again whenever we start fixing it.
>> 
>> 
>>>     2. fix the test to be resistant against implementation changes of selectionModel
>> 
>> Thanks for link to [JDK-8088261](https://bugs.openjdk.java.net/browse/JDK-8088261), As mentioned in this bug
>> description, "Culprit is an incorrect override of select(int): it may reject the new index if that would select a
>> separator, but it must not select an arbitrary index instead",  So It is not sure to me what should `select()` do in
>> such scenario. So I think the test can also go as is, in case we change the behavior then test can be changed with it.
>
>> should be 8088261 (spelling error, I think) - is it okay to change them to the right id?
> 
> That will be good to change, but not sure if as part of this bug. It will be unrelated to fix.

At the end and following your latest comments I did nothing <g> Except adding code comments to the separator test and
the else (toggle nulling) branch.

-------------

PR: https://git.openjdk.java.net/jfx/pull/177