jakartaee / faces

Jakarta Faces
Other
100 stars 55 forks source link

UISelectItems ValueExpression in var attribute throws IllegalArgumentException #1906

Open xmourgues opened 4 months ago

xmourgues commented 4 months ago

Hello,

Since this commit (https://github.com/eclipse-ee4j/mojarra/commit/33c3d06acec968ad9819e92067918ec2096fb5dc) we can no longer set a ValueExpression inside the var attributes of f:selectItems.

This is breaking one of our composite component. The vdl doc (https://jakarta.ee/specifications/faces/4.0/vdldoc/f/selectitems#var) still says that var can take a ValueExpression which should evaluate to a String.

Here is a minimal example :

<?xml version="1.0" encoding="UTF-8"?>
<html xmlns="http://www.w3.org/1999/xhtml" xmlns:h="jakarta.faces.html" xmlns:f="jakarta.faces.core" xmlns:ui="jakarta.faces.facelets"
    xmlns:composite="jakarta.faces.composite">
<composite:interface componentType="compositeSelectOne">
    <composite:attribute name="compositeVar" />
    <composite:attribute name="compositeItemLabel" />
    <composite:attribute name="compositeItemValue" />
</composite:interface>
<composite:implementation>
    <h:selectOneListbox id="firstList" value="...">     
        <f:selectItems value="..." var="#{cc.attrs.compositeVar}" itemValue="#{cc.attrs.compositeItemLabel}" itemLabel="#{cc.attrs.compositeItemValue}" />
    </h:selectOneListbox>   
</composite:implementation>
</html>

Is this a bug or is it really the new intended behavior ? If it is intended, how would one work around this ?

xmourgues commented 3 months ago

Hello again, As a workaround we removed the use of the EL inside the var attribute and forced it to a specific value that we had to impact on every usage of this composite component. It's less sexy, but it works.

So at the very least, the vdl doc should be fixed. Any comment on this ?

volosied commented 3 months ago

I'm sorry for the lack of response. The behavior looks intended to me. It's meant to simply be a name which can be referred to within other value expressions (i.e. the itemValue attribute). We should fix up the vld doc at the least.

@BalusC may offer more insight since he originally implemented this change.

melloware commented 2 months ago

@tandraschko is this something we should do in the PrimeFaces taglib.xml as well?

See: https://github.com/eclipse-ee4j/mojarra/pull/5448

melloware commented 1 month ago

@BalusC can you explain this one more?

I opened PF ticket: https://github.com/primefaces/primefaces/pull/12055

But we are wondering how does removing the type affect these var's? I see OmniFaces and Mojarra do not have a type on the var in their taglibs.