Re: [core] implicit object 'component' in rendered expression (myfaces and spec bug)

[Martin Koci <ma...@gmail.com>]

Hi,


can you review patch for this issue - MYFACES-3157 - please?

Regards,

Kočičák


> Hi,

> more info about this problem:

> 1) I did some testing of mojarra and they do the same in encodeBegin as
> myfaces:
> pushComponentToEL
. if (!isRendered())

> despite of the specification. 

> 2) Specification does not mention pushComponentToEL for encodeAll(),
> only says "If this component returns true from isRendered(), take the
> following action ... Render this component and all its children ...."


> 3) pushComponentToEL and double push:
> component.pushComponentToEL(context, null);
> component.pushComponentToEL(context, null);

> Will be the same component pushed twice on stack?


> Regards, 

> Kočičák

Martin Koci píše v St 25. 05. 2011 v 22:12 +0200:
> Hi,
> 
> there are such cases but not many of them: in myfaces code after quick
> search I guess about 10 such cases. The "main" is in
> UIComponentBase.encodeChildren and in RendererUtils.renderChild (well,
> there was, but I removed it incorrectly as part of MYFACES-3126)
> 
> 
> This affects all attributes of component, not only rendered. If renderer
> encodes children, then must set up context if reads something from
> child. Consider:
> <a:tabbedPane>
>  <a:navigationItem style="#{component.something eq 'b'}" />
> 
> if tabbedPaneRenderer iterates over children directly and reads 'style'
> property, then it must guarantee that 'component' resolves to
> navigationItem otherwise it is incosistent with situation, where
> navigationItem encodes itself.
> 
> This will affects complex structures like dataTable and panelGrid, I
> think.
> 
> Regards,
> 
> Kočičák
>  
> Blake Sullivan píše v St 25. 05. 2011 v 12:42 -0700:
> > I suspect that there are many cases where parent components are looking 
> > at rendered on their children before deciding to traverse into these 
> > children.  If EL-binding rendered to the component is supported, then 
> > the parent is either going to need to stop performing this optimization, 
> > or it is going to have to ensure that the context is setup and torn down 
> > around each call to isRendered.
> > 
> > -- Blake Sullivan
> > 
> > On 5/25/11 11:27 AM, Martin Koci wrote:
> > > Hi,
> > >
> > > for spec I've created bug few days ago:
> > > http://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC-1002
> > >
> > > but I didn't realize how deep impact this bug have: I did a training
> > > about JSF implicit objects in our company and now practically every
> > > coder uses #{component.something} :)
> > >
> > > For myfaces:
> > > https://issues.apache.org/jira/browse/MYFACES-3157
> > >
> > >
> > > Leonardo Uribe píše v St 25. 05. 2011 v 12:36 -0500:
> > >> Hi
> > >>
> > >> I have seen that. The problem is in spec javadoc, the behavior for
> > >> UIComponent.process*
> > >> and encode* is clear about the ordering.
> > >>
> > >> In theory, pushComponentToEL should be called before any call to
> > >> isRendered, but after
> > >> isTransient. Look on MyFaces UIComponentBase.processRestoreState. It has this
> > >>
> > >>      public void processRestoreState(FacesContext context, Object state)
> > >>      {
> > >>          if (context == null)
> > >>          {
> > >>              throw new NullPointerException("context");
> > >>          }
> > >>
> > >>          Object[] stateValues = (Object[]) state;
> > >>
> > >>          try
> > >>          {
> > >>              // Call
> > >> UIComponent.pushComponentToEL(javax.faces.context.FacesContext,
> > >> javax.faces.component.UIComponent)
> > >>              pushComponentToEL(context, this);
> > >>
> > >>              // Call the restoreState() method of this component.
> > >>              restoreState(context, stateValues[0]);
> > >>
> > >> The spec javadoc says the opposite, restoreState should be called "before"
> > >> pushComponentToEL but in practice that breaks UIComponent.EventListenerWrapper.
> > >> I reported the bug long time ago, but it wasn't fixed (I don't know why).
> > >>
> > >> This case is similar. This should be fixed on the spec, but I don't
> > >> see a reason why we shouldn't commit that into myfaces code, because
> > >> after all, nobody relies on the buggy behavior.
> > >>
> > >> I think we should open a new issue on the spec issue tracker:
> > >>
> > >> http://java.net/jira/browse/JAVASERVERFACES_SPEC_PUBLIC
> > >>
> > >> but fix it on myfaces code.
> > >>
> > >> regards,
> > >>
> > >> Leonardo Uribe
> > >>
> > >> 2011/5/25 Martin Koci<ma...@gmail.com>:
> > >>> Hi,
> > >>>
> > >>> <h:form>
> > >>>         <h:panelGroup>
> > >>>                 <h:inputText id="testId"
> > >>>                 rendered="#{component.id eq 'testId'}"                                  value="#{bean.value}" />
> > >>>         </h:panelGroup>
> > >>> </h:form>
> > >>>
> > >>> please notice the expression:
> > >>>
> > >>> rendered="#{component.id eq 'testId'}"
> > >>>
> > >>> that is clearly true.
> > >>>
> > >>> But that does not work as expected: inputText is rendered, but never
> > >>> updates model value.
> > >>>
> > >>>
> > >>> Problem 1.
> > >>>
> > >>> from specification, methods UIComponent.process* and encode*
> > >>> 1) If the rendered property of this {@link UIComponent}
> > >>> false, skip further processing
> > >>> 2) Call {@link #pushComponentToEL}
> > >>>
> > >>> ->  #{component} resolves in rendered="#{}" to parent!
> > >>>
> > >>>
> > >>> Problem 2.
> > >>>
> > >>> MyFaces implement that (pointless) requirement inconsistently: from
> > >>> UIComponentBase.process*:
> > >>>
> > >>> (!isRendered())
> > >>>   return;
> > >>> pushComponentToEL(context, this)
> > >>>
> > >>> and from UIComponentBase.encodeBegin*
> > >>>   pushComponentToEL(context, this);
> > >>>   if (isRendered())
> > >>>
> > >>>
> > >>> causes that example above renderes inputText, but never updates model.
> > >>>
> > >>> Problem 3.
> > >>>
> > >>> RendererUtils.renderChild(FacesContext, UIComponent):
> > >>> in this method it is unappropriate to use following code:
> > >>>
> > >>> if (!child.isRendered()) {
> > >>>             return;
> > >>>         }
> > >>> child.encodeBegin(facesContext);
> > >>>
> > >>> because:
> > >>> 1) it does not take into account pushComponentToEL ( #{component}
> > >>> resolves to parent)
> > >>> 2) behaviour is incosistent with UIComponent.encodeBegin : you'll get
> > >>> "random" rendering - depends if parent of component renders it's
> > >>> children or not! For this case I've created MYFACES-3126, but I'll
> > >>> reopen it now, because simple remove of 'if (!child.isRendered())' does
> > >>> not solve that problem and causes another problem if component
> > >>> getRendersChildren = false;
> > >>>
> > >>>
> > >>> What do yout think about this problem?
> > >>>
> > >>>
> > >>> Regards,
> > >>>
> > >>> Kočičák
> > >>>
> > >>>
> > >>>
> > >
> > 
> > 
> 
> 
>