jsr372-experts@javaserverfaces-spec-public.java.net

[jsr372-experts] Re: [jsr372-experts mirror] Re: Potential issue with hierarchical variable mapper in Facelets

From: Leonardo Uribe <leonardo.uribe_at_irian.at>
Date: Thu, 8 Jan 2015 20:31:54 -0500

Hi

I MyFaces we did a HUGE refactor over that part.

- Add page scope semantic like in jsp
- Add template scope semantic and honor it
- fix c:set
- fix ui:param
.....

It is a long story, but if you are interested, take a look at:

https://issues.apache.org/jira/browse/MYFACES-3169

"... 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. ..."

At the end, the variable mapper logic in MyFaces has a new logic that works
really good. In fact, we did a full in-deep refactor of facelets
template engine,
solving all those issues.

After a lot of trial/error, we have a pretty cool and stable API on MyFaces Impl
(AbstractFaceletContext, FaceletCompositionContext, TemplateContext,
PageContext).

For example instead this:

ctx.getVariableMapper().setVariable(varStr, veObj);

this looks better this:

actx.getPageContext().getAttributes().put(varStr, veObj);

or this:

actx.getTemplateContext().setParameter(names[i], ve);

Instead the old wrapper logic, now there is a default variable mapper
that internally deal with the complexity, taking into account the context.

Fix this part doesn't come with a cost. There are situations where some
code works in Mojarra and not in MyFaces, creating some kind of
"minor incompatibility", that can be easily overcome with some few
lines. But the benefit is huge, because with the change we have solved
a lot of issues.

There is a param in MyFaces 2.2.x:

org.apache.myfaces.STRICT_JSF_2_FACELETS_COMPATIBILITY

By default false, that allows you to switch between the legacy
VariableMapper wrapper approach and the default and more strict,
stable and faster mode.

VariableMapper has a virtue: whatever you put there pass through
everything, even composite components or templates. Useful when
you need a global variable like "locale" or whatever. But it breaks
encapsulation principle. A PageContext / TemplateContext looks
better. In MyFaces we fixed c:set to use PageContext, but if you
use it globally, you can write a custom tag that write on the
VariableMapper directly or use a custom EL Resolver.

Is PageContext / TemplateContext concept useful for component
vendors? I don't know, it looks like an implementation detail
(all tags that requires them are inside JSF Core), but it could be
a couple of valid cases out there (jsp has a PageContext).

I understand this change is not backward compatible. But it is
not a problem for MyFaces users, because we took the step
forward since 2.0. That means component libraries working
with MyFaces will not have any trouble. Only existing apps
that rely on c:set as a way to define "global" or "template" scope
variables.

regards,

Leonardo Uribe

2015-01-08 9:00 GMT-05:00 arjan tijms <arjan.tijms_at_gmail.com>:
> Hi,
>
> On Thu, Jan 8, 2015 at 2:26 PM, Edward Burns <edward.burns_at_oracle.com> wrote:
>>>>>>> On Thu, 8 Jan 2015 12:38:23 +0100, arjan tijms <arjan.tijms_at_gmail.com> said:
>> Ah, welcome to an instance of the biggest challenge we face in evolving
>> JSF. While this may not be correct, it is established behavior, and
>> rather fundamental behavior at that.
>
> Indeed, it goes rather deep and there's perhaps no telling how much
> code (implicitly or explicitly) relies on this.
>
> One additional thing to note though is that a Facelets tag file has
> the conditional shared/not shared variable mapper, while a Facelets
> include never shares. E.g. in
> com.sun.faces.facelets.tag.ui.IncludeHandler.apply(FaceletContext,
> UIComponent) we see:
>
> public void apply(FaceletContext ctx, UIComponent parent) throws IOException {
> String path = this.src.getValue(ctx);
> if (path == null || path.length() == 0) {
> return;
> }
> VariableMapper orig = ctx.getVariableMapper();
> ctx.setVariableMapper(new VariableMapperWrapper(orig));
>
>
>> I'm leery of changing this established behavior.
>
> If a change turns out to be infeasible (it indeed may well be) then a
> far less invasive path to take could be the introduction of some API
> that third party libraries can use to provide a workaround for this.
>
> E.g.
>
> * A simple boolean that indicates if a Facelet got a shared variable
> mapper or not
> * A hook "somewhere" so the process of creating a FaceletContext or
> perhaps even a Facelet itself can be intercepted (so e.g. a
> FaceletContext and/or a Facelet can be wrapped)
>
> In OmniFaces I just worked around this by letting each tagfile "mark'
> the variable mapper and then do some behaviour tests; if it's possible
> for a nested tag to remove the outer marker, we know the variable
> mapper must be shared. It's a bit weird to have behaviour probing code
> in place, and it requires every facelet tag file in an application to
> contain an "o:tagAttribute" tag, but it does work it seems.
>
> See https://github.com/omnifaces/omnifaces/blob/master/src/main/java/org/omnifaces/taghandler/TagAttribute.java#L96
>
> Kind regards,
> Arjan Tijms