On Thu, 22 Jul 2021 19:49:44 GMT, Marius Hanl <mh...@openjdk.org> wrote:
>> This PR fixes multiple NPEs in Choice-and ComboBox, when the selection model >> is null. >> >> ChoiceBox: >> - Null check in **valueProperty()** listener >> >> ComboBox: >> - Null check in **editableProperty()** listener >> - Null check in **valueProperty()** listener >> - Null check in **ComboBoxListViewSkin#updateValue()** >> - Null check in **ComboBoxListViewSkin#layoutChildren()** >> - Null check in **ComboBoxListViewSkin#createListView()** >> >> ~~The tests checks, that no NPE is printed to the console. >> Some checks, that the set value is still displayed (either in the ComboBox >> button cell or the ChoiceBox display label)~~ > > Marius Hanl has updated the pull request incrementally with two additional > commits since the last revision: > > - removed unneeded assert and added comment > - removed unneeded asserts and added another NPE fix. there were two NPEs in createLists - one is fixed, the other not yet :) See the failing test in my comment copying the comment, don't know if you could find it otherwise (any way to un-resolve a review conversation?) >```` > // another test, exposing one of the NPEs in createList > ComboBox<String> comboBox = new ComboBox<>(items); > comboBox.setSelectionModel(null); > installDefaultSkin(comboBox); > > the difference is that setup already installs a skin - so at the time of init > the selectionModel still is available ;) still failing > > The other access is in the listener to listView's selectedIndex .. it might > be a bit tricky to expose in a test, I would go for a combo in a > stage/loader, access the list and change its selectedIndex (maybe needs the > popup open and/or firing a key onto it) good fix and test :) ------------- Changes requested by fastegal (Reviewer). PR: https://git.openjdk.java.net/jfx/pull/557
