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

Leonardo Uribe commented on MYFACES-3169:
-----------------------------------------

VariableMapper strategy still works, so I think you can create a custom tag 
handler (<x:globalParam>?) that set the variable on the VariableMapper just 
like the previous implementation did. It will work without problem.

If you have a param that applies to all views in an application, maybe it is 
better to use an EL Resolver that handle this special name, just like JSF does 
for handle implicit objects ( request, facesContext, externalContext, session, 
application, component, cc, .... ). After all, this is the pattern used for JSF 
to deal with this kind of scenarios.

c:set with var and value properties only define vars on "page" scope, just like 
the old jsp tag did and more according to the spec description. So it works on 
any page, but variables defined will only apply to the page this tag is used.

I know this fix could cause these kind of problems, but in my personal opinion 
it is worst rely on the previous behavior, because that is not clear and does 
not match with the spec documentation. Anyway, I'm open to discussion this 
stuff on dev list if the behavior proposed seems to be very inconvenient.

> ui:param and c:set implementations does not work as expected
> ------------------------------------------------------------
>
>                 Key: MYFACES-3169
>                 URL: https://issues.apache.org/jira/browse/MYFACES-3169
>             Project: MyFaces Core
>          Issue Type: Bug
>          Components: JSR-314
>    Affects Versions: 2.0.7, 2.1.1
>            Reporter: Leonardo Uribe
>            Assignee: Leonardo Uribe
>             Fix For: 2.0.8, 2.1.2
>
>         Attachments: MYFACES-3169-2.patch
>
>
> Original message sent to jsr344-experts and users mailing list:
> Checking how ui:param and c:set works and its relationship with facelets 
> VariableMapper, I notice the original implementation that comes from facelets 
> does not work like a page developer should expect. In fact, in some situations
> ui:param and c:set implementation is just the same.
> The consequence of this situation is there are ways to write code that just 
> should not be allowed, because it breaks the intention of those tags. JSF 
> implementations should fix this, and maybe if it is required add more 
> documentation specifying these tags better.
> The first case is c:set. This is the description on facelets taglibdoc:
> "... Sets the result of an expression evaluation based on the value of the 
> attributes. If 'scope' the is present, but has a zero length or is equal 
> to the string 'page', TagException is thrown with an informative error 
> message, ensuring page location information is saved. ..."
> "... This handler operates in one of two ways.
> 1. The user has set 'var', 'value' and (optionally) 'scope' attributes.
> 2. The user has set 'target', 'property', and 'value' attributes.
> The first case takes precedence over the second ..."
> The buggy behavior of this tag can be seen when it is used in this way:
> <c:set var="someVar" value="someValue"/>
> Look the part that says "... if 'scope' the is present, but has zero length or
> is equal to the string 'page' ..." . In JSP, it exists a page context and
> in that context, the variable has scope to the current page. Since in that
> case there are no template tags, this variable cannot be located on other 
> pages included with jsp:include or whatever you use. 
> The current implementation of c:set that comes from facelets 1.1.x does not
> implements the original intention of this tag in jsp, and instead it uses
> VariableMapper instance obtained through FaceletContext (which is instance
> of ELContext). Since both JSF 2.0 implementations comes from the original 
> facelets 1.1.x code, you can see the following problems: 
> cset1.xhtml
> <c:set var="someVar" value="someValue"/>
> <ui:decorate template="cset1_1.xhtml">
>    <!-- ... -->
> </ui:decorate>
> cset1_1.xhtml
> <ui:composition>
>    <h:outputText value="#{someVar}"/>
> </ui:composition>
> The previous code in practice will output "someValue", but it should not, 
> because "someVar" should be only available on the current .xhtml page, in
> this case cset1.xhtml. 
> Now consider this more realistic example:
> cset2.xhtml
> <c:set var="someVar" value="someValue"/>
> <ui:decorate template="cset2_1.xhtml">
>    <!-- ... -->
>    <ui:define name="header">
>        <h:outputText value="#{someVar}"/>
>    </ui:define>
> </ui:decorate>
> cset2_1.xhtml
> <ui:composition>
>    <c:set var="someVar" value="badValue"/>
>    
>    <!-- ... something with someVar ... -->
>    
>    <ui:insert name="header"/>
> </ui:composition>
> In practice the output will be "badValue", but it should be "someValue",
> again because c:set intention is to define a value that should be
> available only on the current page. This problem is also valid if you
> replace ui tags with a composite component and cc:insertXXX tags.
> Now take a look at this one:
> cset3.xhtml
> <c:set var="someVar" value="someValue"/>
> <ui:decorate template="cset3_1.xhtml">
>     <!-- ... code without use ui:param ... -->
> </ui:decorate>
> <h:outputText value="#{someVar}"/>
> cset3_1.xhtml
> <ui:composition>
>    <c:set var="someVar" value="badValue"/>
>    <!-- ... something with someVar ... -->
> </ui:composition>
> In practice the output will be again "badValue", but it is interesting to note
> that if you use ui:param, the example will work again, because a
> VariableMapperWrapper is used, discarding the bad value after ui:decorate 
> ends.
> Things start to get worse when you see how ui:param works:
>     String nameStr = this.name.getValue(ctx);
>     ValueExpression valueVE = this.value.getValueExpression(ctx, 
> Object.class);
>     ctx.getVariableMapper().setVariable(nameStr, valueVE);
>         
> It is the same thing as c:set!!!!! . 
>     if (this.scope != null)
>     {
>         /* ... Does this exception really has sense ??? ... */
>         if ("page".equals(scopeStr))
>         {
>             throw new TagException(tag, "page scope is not allowed");
>         }
>         /* ... some stuff that works well ...*/
>     } else {
>         ctx.getVariableMapper().setVariable(varStr, veObj);
>     }
> Why this code hasn't been broken before? because nobody has ever use c:set and
> ui:param with exactly the same var name. Maybe because on facelets dev 
> documentation:
> http://facelets.java.net/nonav/docs/dev/docbook.html
> says this:
> "... Table 3.5. <c:set/> (Avoid if Possible) ..."
> Really there are some particular situations where c:set is useful.
> There are a lot more examples I tried on ui:param that just don't work. But
> the big question is how c:set and ui:param should work?
> - c:set using only 'var' and 'value' should define the variable only as 
> "page" scoped, just like the old jstl tag does and the current spec javadoc
> says.
> - ui:param should define a variable "template" scoped, that means it applies
> to both template client and templates. It should be propagated through 
> ui:composition, ui:decorate, ui:define and ui:insert, but it should not pass
> composite components, because this one defines a different template resolution
> context (hack only on MyFaces, but valid for JSF). It should not pass on
> nested templates case (nested ui:composition or ui:decorate). 
> - The facelets taglibdoc of ui:param says:
> "... Use this tag to pass parameters to an included file (using ui:include), 
> or a template (linked to either a composition or decorator). Embed ui:param 
> tags in either ui:include, ui:composition, or ui:decorate to pass the 
> parameters. ..."
> JSF implementation should do exactly what it says here.
> Note all this should work using a more elaborate hack over VariableMapper,
> because it is not possible to use another alternative here. One idea is 
> create a component that defines a "local" page scope just like {} does 
> in java code, but maybe is too much for something than in practice should
> be simple.
> I think this should be fixed. Obviously I'm doing an interpretation of the
> few documentation available, but note at the end a "final word" should be
> done here at spec level to keep compatibility between JSF implementations.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira

        

Reply via email to