RFR: 8089398: [ChoiceBox, ComboBox] throws NPE on setting value on null selectionModel [v3]

Jeanette Winzenburg fastegal at openjdk.java.net
Thu Jul 8 12:01:57 UTC 2021


On Thu, 8 Jul 2021 11:19:57 GMT, Ajit Ghaisas <aghaisas at openjdk.org> wrote:

> > the first two are naturally within the original scope, the third is near enough (a property on one of the covered controls) to be included .. the last is arguable, IMO - would tend to not include it here but open a follow-up to then include _all_ sync issues in the skin (probably needs more digging and definitely more testing).
> > Thoughts?
> 
> I reviewed the files and found those two places in files being edited in this PR were missing the newly added null check. It is logical to add null checks in all places in files being edited in this PR.

good point, except that it's not _all_ in the skin *grinning (there are two in listeners to listView selectionModel properties)

> Fixing review comments in the same PR or creating a follow-up issue is up to the author and reviewer's agreement.
> Let us not undo the change which is already done in this PR :)
> 

I like the _not undoing_ aspect :) - so the question is: should we find  and fix all in combo's skin in this issue or leave this as-is and create a follow-up for the not yet included? 

> If we are sure that the similar fix is needed at other places (apart from the files which are being fixed in this PR), we can do it in a single OR multiple follow-up issue(s).

probably was unclear: didn't mean other controls - suspect there are, which we wait to bubble up sometime :)

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

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


More information about the openjfx-dev mailing list