Collect: NullPointerException in SelectOneListAdapter.java:105

[grzesiek2010]

Software and hardware versions

Collect v1.22.1 and older versions

Problem description

Fatal Exception: java.lang.NullPointerException: Attempt to invoke virtual method 'boolean java.lang.String.equals(java.lang.Object)' on a null object reference at org.odk.collect.android.adapters.SelectOneListAdapter.setUpButton(SelectOneListAdapter.java:105) at org.odk.collect.android.adapters.SelectOneListAdapter.setUpButton(SelectOneListAdapter.java:38) at org.odk.collect.android.adapters.AbstractSelectListAdapter$ViewHolder.bind(AbstractSelectListAdapter.java:216) at org.odk.collect.android.adapters.SelectOneListAdapter$ViewHolder.bind(SelectOneListAdapter.java:90) at org.odk.collect.android.adapters.AbstractSelectListAdapter.onBindViewHolder(AbstractSelectListAdapter.java:82) at org.odk.collect.android.adapters.AbstractSelectListAdapter.onBindViewHolder(AbstractSelectListAdapter.java:60)

Steps to reproduce the problem

Try to use this form:
selectOne.xml.txt

[getodk-bot]

Hello @grzesiek2010, you have been unassigned from this issue because you have not updated this issue or any referenced pull requests for over 15 days.

You can reclaim this issue or claim any other issue by commenting @opendatakit-bot claim on that issue.

Thanks for your contributions, and hope to see you again soon!

[lognaturel]

@seadowg has pointed out that this happened a lot a couple of days ago. Firebase crash link (needs auth). What likely happened is that there is a form out there with an empty choice and that the users of that form tried over and over again to open it.

@ggalmazor put in https://github.com/opendatakit/javarosa/pull/533 which will fix the crash. When a choice with an empty value is read from a serialized form def, it won't be null.

However, this still means that a user can select a choice that will not actually save a value which is what @grzesiek2010 was trying to address at #3143.

My current ideal would be for choices with null values to still show up but not be selectable. When a user selected one, it would unselect and a toast would say "This choice has no value and can't be selected" or something like that. I think that @grzesiek2010 wanted to avoid going down this route because each select appearance would need to do this separately, is that correct?

[grzesiek2010]

each select appearance would need to do this separately, is that correct?

yes that would require handling it in a few different places.

[seadowg]

@lognaturel and I chatted about this recently and felt we can probably do without a toast warning unless we start seeing feedback around this issue. So given @ggalmazor's changes it feels that (once we upgrade JavaRosa) we should just make sure we have a test at the Widget/Adapter level to make sure that rendering/selecting choices with empty values doesn't explode. Does that sound right @grzesiek2010 / @lognaturel? I can look at adding that if so.

[lognaturel]

@seadowg That sounds fine to me. Can we also add a filteredItems.get(index).getValue() != null in front of filteredItems.get(index).getValue().equals(selectedValue)? It should generally be possible for Collect to handle null values.

[seadowg]

@lognaturel that sounds good

[seadowg]

@lognaturel do we need a new JavaRosa release to get the fix?

[getodk-bot]

Hello @seadowg, you have been unassigned from this issue because you have not updated this issue or any referenced pull requests for over 15 days.

You can reclaim this issue or claim any other issue by commenting @opendatakit-bot claim on that issue.

Thanks for your contributions, and hope to see you again soon!

[lognaturel]

I continue to think that #3752 is right and good to have. The implication will be that if there is a choice with a null value, it will be impossible to select.

I had been thinking that in fact perhaps https://github.com/opendatakit/javarosa/pull/533 should be reverted but I don't know anymore. The reason I was thinking that is that nullIfEmpty is used for virtually everything that goes through the serialization process in JavaRosa.

The implication of the change in JavaRosa is that after #3752 is merged, I believe that on form re-entry the first choice with an empty value will appear selected in Collect if a choice with an empty value was selected. So it would look like a choice was successfully made but at time of analysis, it would be impossible to distinguish between that selection and leaving the question blank. I'm not totally clear on what would happen with required checks. It might be better to leave nullIfEmpty and also use a null value on the initial parse.

But anyway. I think we've spent enough time on this. There won't be a crash and it will be up to users to figure out what they meant by an empty choice value. I wish we could do a toast or something on selection but I understand why it's not practical. If this were common, someone would have brought it up by now.