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

2011-06-18 Thread Martin Koci
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 Kocimartin.kocicak.k...@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 

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

2011-05-26 Thread Martin Koci
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 Kocimartin.kocicak.k...@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, 

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

2011-05-25 Thread Leonardo Uribe
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 martin.kocicak.k...@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





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

2011-05-25 Thread Martin Koci
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 martin.kocicak.k...@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
 
 
 
 




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

2011-05-25 Thread Blake Sullivan
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 Kocimartin.kocicak.k...@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









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

2011-05-25 Thread Martin Koci
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 Kocimartin.kocicak.k...@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