Found in GWT Release (e.g. 1.5.3, 1.6 RC): 2.1RC1

Detailed description (please be as specific as possible):

ValueListBox#updateListBox should handle a null value. Right now there is no null check and if you call setAcceptableValues() for example before setting the (current) value, updateListBox() adds a null/dummy entry to the listbox (or throws a NPE in your key provider or renderer).

Workaround if you have one:

Add:

if (value == null) return;

as first line in alueListBox#updateListBox()

I'm really surprised this has been open for a year with no update. It seems counterintuitive that you have to set the value on the listbox before setting the "acceptable" values.

Uploaded a patch to rietveld:

http://gwt-code-reviews.appspot.com/1619803/

ValueListBox ensures that any selected value is part of the list of acceptable values, even if that means adding it to that list so it can be selected.

If 'null' is not part of your list of acceptable values, you have to call setValue to set the default value. The behavior you're experiencing is simply that the default "default value" is 'null'.

I admit this can be a bit counter-intuitive, but if something has to be done, it's either about: - adding a overload to setAcceptableValues to pass the default value (setAcceptableValue(values, defaultValue)) - or changing setAcceptableValues' behavior to automatically select the first value in the list The issue specifically is not in updateListBox.

On second thought, changing the behavior of setAcceptableValues could break a lot of people, as I believe the current behavior was designed to support the use case of acceptable values loaded asynchronously, and you might already have the selected value when doing the request for the acceptable values, or you load it asynchronously too and then you might call setValue before setAcceptableValues (or the other way around, without knowing in advance). In case setAcceptableValues includes the 'null' value, it'd "just work"; otherwise it requires a bit more code if you don't want to have the 'null' value automatically added to the list.

I'm not quite following, Thomas. I agree this does change the behavior if:

The user was calling setAcceptableValues(a, b) then later (sync or async) called setValue(...), but they actually liked the side-effect of the list box now having acceptable values of empty, a, b in it.

Is this the scenario you meant by "break a lot of people"?

AFAIK if you load acceptableValues async, but set the value sync, the proposed patch/change should not change the existing behavior.

otherwise it requires a bit more code if you don't want to have the 'null' value automatically added to the list

Isn't that what the proposed patch does? null isn't added unless you explicitly request it (either via acceptable values or setValue).

Thomas, I'm sure that the current scenario break a lot of people ;)

Sorry Stephen, I had that comment unsaved in my browser for a whole night, and thought I'd save it anyway (even after our discussion on the code review), as a "public update" (users read the issue tracker, not code reviews).

I was talking about "changing setAcceptableValues' behavior to automatically select the first value in the list", unconditionally. You proposed an "uninitialized" state in your patch, which is a third proposal: "changing setAcceptableValues' behavior to automatically select the first value in the list if one hasn't yet been explicitly set", and which I believe is the most sensible change we could make without breaking everyone.

@stanislav: the current behavior is counter-intuitive for sure, but completely predictable and well-defined: any call to setValue or setAcceptableValues will update the list box with the union of the current value and acceptable values, and ensure the current value is selected. The value defaults to null, there's no "uninitialized" state pending a call to either setValue or setAcceptableValues.

Stephen proposes adding such an "uninitialized" state, so that the "current value" is uninitialized (and not 'null') until either setValue or setAcceptableValues is called (now the question is: should it be "either setValue or setAcceptableValues", or only "setValue"? with the "uninitialized value" being the first acceptable value, or null if no acceptable values have been set yet).

Issue 7388 has been merged into this issue.

@t.broyer @stephen.haberman can we include this into 2.5.1?

Comment deleted

Luis: setAcceptableValues(Collections.emptyList()) ? But what would you want to clear the acceptable values? You'd rather want to set new acceptable values, and if you don't want to give any choice to the user then use Collections.singleton(null), but at any time getValue() needs to return some value, so there needs to be at least one value to "choose" from.

2.5.1 should be released in the next few weeks.

@thomas,

seems like your comment should go somewhere else?!

I was replying to comment 10 which his author has now deleted.

Whats the status here?

Wouldn't it be possible to allow both behaviors by introducing setAcceptableValuesStrict(Collection values) //resets 'current value' to first entry of the new 'values' if 'current value' is not in the new list of 'values' setAcceptableValuesStrict(Collection values, T select) //uses 'select' as new 'current value'. 'select' must be available in 'values' otherwise an exception will be thrown.

Then modify setValue(T value) a bit so it throws an exception if setAcceptableValuesStrict() has been used to fill the box and the new value is not in the list of acceptable values.

Only thing thats a bit unclear is if setAcceptableValuesStrict() should fire ValueChangeEvents if the change the current value behind the scenes. Maybe we would also need

setAcceptableValuesStrict(Collection values, boolean fireEvents) setAcceptableValuesStrict(Collection values, T select, boolean fireEvents)

That way we can choose the desired behavior.

@stephan any chance of addressing that with a patch in gerrit?

Sure, I'll take a look at moving it over...

Issue tracked moved to github, see https://github.com/gwtproject/gwt/issues