Hi, The patch looks good, but actually I did not test it myself. Have you also tried it with enum values?
Funny that the TCK does not fail here, because the spec explicitly tells us to use coercion. If it works well with enums, +1 on committing this patch. Regards, Jakob 2010/9/14 Martin Koci <martin.kocicak.k...@gmail.com>: > Hi, > > please review https://issues.apache.org/jira/browse/MYFACES-2920 > > 1) JavaDoc explicitly mentions EL coecion but not Converter: > https://javaserverfaces.dev.java.net/nonav/docs/2.0/javadocs/javax/faces/component/UISelectOne.html#validateValue%28javax.faces.context.FacesContext,%20java.lang.Object%29 > > "Before comparing each option, coerce the option value type to the type > of this component's value following the Expression Language coercion > rules" - > > but obviously if Converter was used for SelectItem.value -> String > should be used for String -> SelectItem.value > > 2) EL coercion problem: yes, that can happen: > > > <h:selectOne value="#{bean.floatProperty}"> > <f:selectItem itemValue="" .../> > > > Selected value will be 0.0 in this case. This is probably not problem if > property is primitive float but with Object Float user expects null > (maybe). But is itemValue="" permitted at all? > > > 3) Trinidad: Matthias, isn't this change related to this problem too? : > http://svn.apache.org/viewvc/myfaces/trinidad/trunk/trinidad-impl/src/main/java/org/apache/myfaces/trinidadinternal/renderkit/core/xhtml/SimpleSelectOneRenderer.java?r1=672549&r2=694485&diff_format=h > > > Regards, > > Kočičák > > > Martin Marinschek píše v Pá 27. 08. 2010 v 17:50 +0200: >> I didn't totally think through if it is usable in this case, but more >> general, if you can prevent it, do not use EL coercion. Why? >> >> As specified, EL coercion will coerce null to 0 or an "". For the next >> version of the EL, this behaviour might be configurable. >> >> best regards, >> >> Martin >> >> On 8/27/10, Jakob Korherr <jakob.korh...@gmail.com> wrote: >> > Hi, >> > >> > Hmm, could be. I guess this code was introduced in JSF 1.1 (no unified EL >> > and thus no EL coercion available) and never changed in later versions. >> > >> > If you can make the scenario work using EL coercion, please file an issue >> > in >> > the JIRA. Thanks! >> > >> > Regards, >> > Jakob >> > >> > 2010/8/27 Martin Koci <martin.k...@aura.cz> >> > >> >> Hi, >> >> >> >> >> >> I understand that requirement but isn't it EL coercion task ? : >> >> >> >> https://javaserverfaces-spec-public.dev.java.net/issues/show_bug.cgi?id=152 >> >> >> >> and from UISelectMany.validateValue JavaDoc: >> >> >> >> " ...Before comparing each option, coerce the option value type to the >> >> type of this component's value following the Expression Language >> >> coercion rules ..." >> >> >> >> >> >> Regards, >> >> >> >> Martin Kočí >> >> >> >> Jakob Korherr píše v Pá 27. 08. 2010 v 15:31 +0200: >> >> > Hi Martin, >> >> > >> >> > The purpose of this conversion is that the value of the SelectItems >> >> > may be a String, but the real (already converted) value of the >> >> > UISelectMany may be something else, e.g. Float. Imagine the following >> >> > scenario: >> >> > >> >> > <h:selectManyCheckbox value="#{myBean.inputFloatArray}"> >> >> > <f:selectItem itemValue="1.1" /> >> >> > <f:selectItem itemValue="1.2" /> >> >> > <f:selectItem itemValue="1.3" /> >> >> > </h:selectManyCheckbox> >> >> > >> >> > The itemValues of all SelectItems are Strings, but the UISelectMany >> >> > points to a property which is of type Float[]. Now because of the fact >> >> > that all Strings can be converted into Floats, this scenario must >> >> > work. >> >> > >> >> > If you now take a look at _SelectItemsUtil.matchValue(), you can see >> >> > that not the component's value, but the value of the SelectItem is >> >> > converted via the _ValueConverter: >> >> > >> >> > SelectItem item = selectItemsIter.next(); >> >> > Object itemValue = item.getValue(); >> >> > if(converter != null && itemValue instanceof String) >> >> > { >> >> > itemValue = converter.getConvertedValue(context, >> >> > (String)itemValue); >> >> > } >> >> > >> >> > and then matched against the real value (again which is e.g. Float): >> >> > >> >> > if (value==itemValue || value.equals(itemValue)) >> >> > { >> >> > return true; >> >> > } >> >> > >> >> > Furthermore the converter has to be invoked with the wrapped String in >> >> > a String[], because UISelectMany.getConvertedValue() needs a >> >> > submittedValue of type String[]. >> >> > >> >> > Is this clear for you or should I explain it in more detail? >> >> > >> >> > Regards, >> >> > Jakob >> >> > >> >> > 2010/8/27 Martin Koci <martin.k...@aura.cz> >> >> > Hi, >> >> > >> >> > >> >> > what is the purpose of _SelectItemsUtil._ValueConverter in >> >> > UISelectMany.validateValue(FacesContext, Object) ? >> >> > >> >> > Two weird things: >> >> > >> >> > 1) it wraps value into new String[] { value } >> >> > 2) it calls UISelectMany.this.getConvertedValue and it leads >> >> > to >> >> > Rendered.getConvertedValue call >> >> > >> >> > I don't see sence in call of >> >> > UISelectMany.this.getConvertedValue >> >> > because this already done: we are in method validateValue here >> >> > and >> >> > conversion with Renderer.getConvertedValue is done already. >> >> > This causes >> >> > calls to Renderer.getConvertedValue during process validation >> >> > which is >> >> > unintented I think. >> >> > >> >> > >> >> > Regards, >> >> > >> >> > Martin Kočí >> >> > >> >> > >> >> > >> >> > >> >> > >> >> > -- >> >> > Jakob Korherr >> >> > >> >> > blog: http://www.jakobk.com >> >> > twitter: http://twitter.com/jakobkorherr >> >> > work: http://www.irian.at >> >> >> >> >> >> >> > >> > >> > -- >> > Jakob Korherr >> > >> > blog: http://www.jakobk.com >> > twitter: http://twitter.com/jakobkorherr >> > work: http://www.irian.at >> > >> >> >> -- >> >> http://www.irian.at >> >> Your JSF powerhouse - >> JSF Consulting, Development and >> Courses in English and German >> >> Professional Support for Apache MyFaces >> > > > -- Jakob Korherr blog: http://www.jakobk.com twitter: http://twitter.com/jakobkorherr work: http://www.irian.at
