myfaces-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Martin Kočí (JIRA) <...@myfaces.apache.org>
Subject [jira] [Commented] (MYFACES-3169) ui:param and c:set implementations does not work as expected
Date Sun, 19 Jun 2011 13:39:47 GMT

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

Martin Kočí commented on MYFACES-3169:
--------------------------------------

hmm, we have many (over 1500!) views where ui:param is used. Structure looks:
<ui:decorate xmlns:ui="..."
    template="Template1.xhtml">

  <ui:param name="localization" value="#{aSpecificLocalizationSource}" />
  <ui:define name="content">
   <ui:include src="TemplateClient2.xhtml" />

 </ui:define>

TemplateClient2.xhtml itself uses another template2.xhtml and uses ui:include etc.

The point is that all parts this facelet use variable "localization". 

Now with this change, this stops working and included parts of facelets do not resolve this
variable. I agree that ui:param is was misused here and relied on internal VariableMapper
- usage and c:set as "page scope" variable is better.

But simple replacing ui:param with c:set does not solve this problem, because DecorateHandler
support only define and param! 

How to solve this? Please notice that ui:decorate is root element and has ui:param inside,
not outside the ui:decorate. 

> 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

       

Mime
View raw message