New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SelectItemIndexConverter getAsObject should test null on the submitted value #436

Closed
xqvier opened this Issue Feb 12, 2018 · 3 comments

Comments

Projects
None yet
2 participants
@xqvier

xqvier commented Feb 12, 2018

All is in the title.

On WAS 8.5.5.11, the getAsObject method is called even when the submittedValue is null.
This is the case when using a <f:selectItem noSelectionOption="true" />.
The implem used is myfaces 2.0.2

The method should test beforehand :

if (pValue == null || pValue.isEmpty()) {
    return null;
}
@BalusC

This comment has been minimized.

Member

BalusC commented Feb 17, 2018

With SelectItemsIndexConverter, an empty submitted value in getAsObject() is totally unexpected as the getAsString() normally doesn't return an empty string for a correct list of select items. It always returns the index value, also for empty item values.

Try a newer MyFaces version to exclude one and other.

By the way, please make sure that you correctly understand the intent of noSelectionOption. It's one of the most misunderstood attributes: https://stackoverflow.com/q/11360030/157882

@xqvier

This comment has been minimized.

xqvier commented Feb 23, 2018

Actually, I'm forced to use this myfaces version as it is the one shipped with Websphere Application Server 8.5.5.11 ND (and still the one in the latest 8.5.5.13 but i have to be strictly compatible to the 8.5.5.11) and upgrading WAS provided library is for me a struggle (i'm still trying as there is a memory leak issue in that myfaces version).

I know this is a myfaces 2.0.2 issues and that it seems resolved on later 2.0.x release (just guessing from the source code) but I opened this issue not because it's blocking us (I overrode it anyway) but just trying to make your library fill that compatibility matrix as it's really usefull and people could benefits from it. I've noticed in some part of your library that you fix some other library issue within your code without telling us to upgrade that said library :p
If you're filling lazy I could also maybe do a MR but I think you'd lose more time telling me the comment isn't exactly as you like it than actually making the change.
You can close it if you consider it out of scope. (no hard feelings :)

As for the noSelectionOption, I think I'm using in its right context : using JSF2 and specifying an option that means no-option has been selected (which should result in a null in the backing bean)

BalusC added a commit that referenced this issue Mar 24, 2018

@BalusC

This comment has been minimized.

Member

BalusC commented Mar 24, 2018

Added to 2.6.9-SNAPSHOT

@BalusC BalusC closed this Mar 24, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment