You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by "Leonardo Uribe (Commented) (JIRA)" <de...@myfaces.apache.org> on 2012/04/13 03:47:21 UTC

[jira] [Commented] (MYFACES-3520) False evaluation of variables/params with the same name

    [ https://issues.apache.org/jira/browse/MYFACES-3520?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13253047#comment-13253047 ] 

Leonardo Uribe commented on MYFACES-3520:
-----------------------------------------

The problem with this one is c:forEach relies on VariableResolver, and that is a bad idea by some reasons:

- It can "pass" across everything, including other templates and composite components and breaks encapsulation principle.
- The value is statically stored into the inner value expressions, and that means they can't be cached and reused across pages.

My advice is replace c:forEach with t:dataTable or t:dataList with rowStatePreserved="true". They are more flexible as described in:

http://lu4242.blogspot.com/2011/06/jsf-component-state-per-row-for.html

Additionally, avoiding c:forEach eliminates the effect over state size. 

Unfortunately, after a lot of changes done in myfaces core, the conclusion is previous c:forEach behavior is not fixable. It is possible to use the template context map to store the var and disable EL expression caching, but anyway c:forEach is too inconvenient.

Change the order of VariableResolver stuff is reasonable but is also not wanted. In fact, remove any local VariableResolver usage inside facelets algorithm is preferred, so it is just there for backward compatibility. Note myfaces changed the way c:set and ui:param works to match better the spec.

So, the question is if we change c:forEach to use "template context scope" instead. Sounds reasonable. c:forEach var should not pass through ui:include or composite components. In this case, ui:include creates another template context, so with that the problem is fixed, and since the expression is still resolved through VariableResolver, the change will not break anything.
                
> False evaluation of variables/params with the same name
> -------------------------------------------------------
>
>                 Key: MYFACES-3520
>                 URL: https://issues.apache.org/jira/browse/MYFACES-3520
>             Project: MyFaces Core
>          Issue Type: Bug
>    Affects Versions: 2.1.6
>            Reporter: dennis hoersch
>
> I have an scenario where an xhml is included. The included file has a parameter with the same name as some outer variable. In the included file the parameter is ignored and the outer variable is used. 
> Some xhtml:
> <c:forEach var="item" begin="1" end="3">
>     <div>
>         <ui:include src="templateContextTestInclude.xhtml">
>             <ui:param name="item" value="#{item + 10}" />
>         </ui:include>    
>     </div>
> </c:forEach> 
> templateContextTestInclude.xhtml:
> <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" 
> "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
> <html xmlns="http://www.w3.org/1999/xhtml"
>     xmlns:ui="http://java.sun.com/jsf/facelets"
>     xmlns:h="http://java.sun.com/jsf/html"
>     xmlns:f="http://java.sun.com/jsf/core"
>     xmlns:c="http://java.sun.com/jsp/jstl/core"
>     xmlns:t="http://myfaces.apache.org/tomahawk">
>     <body>
>         <ui:composition>        
>             Item: <h:outputText value="#{item}" /><br/>       
>         </ui:composition>
>     </body>
> </html>  
> I found out that the "hierarchy" of VariableMappers is asked for a value before the 'DefaultVariableMapper' tests the current template/page context. That was not what I expected. Is that how it should be?
> In JSF1 it works that way.
> (If I change the VariableMapperWrapper locally to test the template/page context first the behaviour is as I would have expected (don't know if it is the right place ;-)):
>     public ValueExpression resolveVariable(String variable)
>     {
>         
>         AbstractFaceletContext faceletContext = (AbstractFaceletContext) FacesContext.getCurrentInstance().getAttributes().get(FaceletContext.FACELET_CONTEXT_KEY);
>         
>         //Check on page and template context
>         PageContext pageContext = faceletContext.getPageContext(); 
>         if (pageContext != null && pageContext.getAttributeCount() > 0)
>         {
>             if (pageContext.getAttributes().containsKey(variable))
>             {
>                 ValueExpression returnValue = pageContext.getAttributes().get(variable);
>                 if (_trackResolveVariables)
>                 {
>                     _variableResolved = true;
>                 }
>                 return returnValue;
>             }
>         }
>         
>         TemplateContext templateContext = faceletContext.getTemplateContext();
>         if (templateContext  != null && !templateContext.isParameterEmpty())
>         {
>             if (templateContext.getParameterMap().containsKey(variable))
>             {
>                 ValueExpression returnValue = templateContext.getParameter(variable);
>                 if (_trackResolveVariables)
>                 {
>                     _variableResolved = true;
>                 }
>                 return returnValue;
>             }
>         }
>         
>         ValueExpression ve = null;
>         ....
> )
> Thanks in advance,
> dennis

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira