ComboBox fires a valuechange event when selecting an item that has an empty string as a value

[FollowSteph]

Description of the bug

This is can be replicated (and I'm linking my github repo with the code to replicate it) where if you setup the combobox with objects and one of them has an empty toString value then it will somehow force it to be null. This seems to be independent of whether a label renderer is defined or if the object has a value, the equals, and so on. In other words the selected value is set through the toString() method which is wrong. In fact if two values have the same toString() value then both items in the combobox are selected.

This is very bad because what if you have two john smith's that have say a different database id?

I've been able to consistently re-create the issue which you can view in my github repo at: https://github.com/FollowSteph/vaadin-test-todo which just extends the default todo sample app on the main vaadin website.

Specifically the MainView class sets up the bug by just adding a combobox. I've added a valueChangeListener which is called without any user interactions to set the value to null if a person with a name of "" is selected. If you change the list from two "" to two "Bob" then you'll find that both Bob are selected, even if you change the equals or hashcode methods so that it only compares the id instead of the name you get the same issue.

In other words the combobox relies on the toString() of the objects for everything which is wrong because you could have two people with the same name for example.

Expected behavior

The combobox should not fire a value change to null if there are two duplicate toString() values on the objects in the combobox. If you have two john smith in a list of people then it should pick the one you selected even if it may not be obvious which john is which instead of firing setValue(null).

Minimal reproducible example

https://github.com/FollowSteph/vaadin-test-todo

Versions

• Vaadin / Flow version: 24.1.2
• Java version: 20.0.1
• OS version: Windows 11
• Browser version (if applicable):
• Application Server (if applicable):
• IDE (if applicable):

[knoobie]

Could be related #5184

[mcollovati]

The issue seems related to the one mentioned by @knoobie.
Just want to mention that setting an ItemLabelGenerator that produces different labels for every entry, the combo box works correctly, and the correct Java object is passed to the listener.

[FollowSteph]

I just added ItemLabelGenerator to the sample code in the github repo link and it still has the same issue. In other words this doesn't resolve the issue and it's still using the toString() method.

[mcollovati]

As I mentioned, it works if the ItemLabelGenerator generates a different value for every item, e.g. personComboBox.setItemLabelGenerator(person -> person.getName() + " - " + person.getId());

[FollowSteph]

Ah. My apologies. I misread your comment.

[sissbruecker]

From testing this, I can see two issues:

• After selecting an item that has an empty string as label in the overlay, blurring the field results in the combo box setting the value to null
• After selecting an item that has a duplicate label in the overlay, blurring the fields results in the combo box selecting the first item with that label

While the field has focus everything works fine - selecting items in the overlay keeps the same item selected, even if the label is empty or is a duplicate. The issue only occurs once you blur the field, at which point the component reevaluates which item is selected, and then picks the first available match, or null if the input value is empty, basically ignoring which item you have manually selected in the overlay.

While using an empty string or duplicate values as label is really not advisable, this could be considered a bug in that the item that has been explicitly selected in the overlay is disregarded, even though the input value didn't change between selecting the item and blurring the field.

[TatuLund]

While using an empty string or duplicate values as label is really not advisable, this could be considered a bug in that the item that has been explicitly selected in the overlay is disregarded, even though the input value didn't change between selecting the item and blurring the field.

@sissbruecker could this issue be addressed by this? #5712

Although this leaves out one more use case. Consider you have duplicate item label, if that case is fixed by introduction of dirty state and value is not validated and repicked if text field is not touched, but if user changes the value to something and back to original, I assume the field to be dirty and to be revalidated again.

[FollowSteph]

If I may interject, and I understand what you are saying from a UI perspective, but from a coding perspective the toString() method may not always return a unique value, nor should it really be used as ID because that's not it's intent. The API states: "Returns a string representation of the object." and further adds "In general, the toString method returns a string that "textually represents" this object. The result should be a concise but informative representation that is easy for a person to read." In other words it really shouldn't be used as an ID.

In a UI perspective, such as a ComboBox it makes sense that each item is unique, but the toString() isn't really meant to be a unique ID. If anything the hashCode() method would be a better choice, but even then I would possibly recommend an id generator like the label generator because in many cases there will be a database ID or something or something along those lines and it would be very useful to use. Worse case a hashcode would be a better option in my view. It's important to keep in mind that the toString() method can be used outside of the UI and for other purposes such as a reporting template engine, and so on, where you want the toString() method to say just display a person's name (and there could be more than one John Smith for example).

Just to add I suspect a lot of people create a combobox of people/person type objects and just want the name to be displayed in the combobox forgetting that there can be more than one person with the same name. It's a common thing to forget. The challenge then is that if the object representing the person uses the toString() in a way to textually represent the person as is suggested by the Java API then we're back at this issue where the toString() may not work. It could have duplicates or blanks. Yes from a UI perspective that should be cleaned up, and that was the goal with the itemLabelGenerator, but that doesn't mean the toString() also needs to be unique because again it can be used in other places in the code and needs to rely on a textual representation.

If I misread the thread then my sincerest apologies.