On Thu, 16 Apr 2020 11:23:24 GMT, Ambarish Rapte <ara...@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
