[ https://issues.apache.org/jira/browse/MYFACES-3169?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13051058#comment-13051058 ]
Martin Kočí commented on MYFACES-3169: -------------------------------------- I have problem with NPEs and it's probably caused by: new TemplateManagerImpl(null, INITIAL_TEMPLATE_CLIENT, true, INITIAL_PAGE_CONTEXT) the first param is null - is that ok? Docs says "dummy template client" but can be Facelet owner null really? With complex template-based view I'm getting this NPE: java.lang.NullPointerException at org.apache.myfaces.view.facelets.impl.DefaultFaceletContext.getExpressionFactory(DefaultFaceletContext.java:218) because null is propagated from TemplateManagerImpl to DefaultFaceletContext > 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