You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@myfaces.apache.org by Martin Marinschek <ma...@gmail.com> on 2005/01/24 01:09:00 UTC

Re: cvs commit: incubator-myfaces/src/share/org/apache/myfaces/renderkit/html HtmlRendererUtils.java

Hi Sylvain,

I tried to resolve a problem with validation not happening in some
cases with that lines - so I believe these lines are essentially
correct.

The problem lies in the fact that obviously something is decoded which
shouldn't be decoded (and I mean that the parent of the component is
calling the decode on the component even though it should not cause
the component is not visible).

Can you tell me again with which component you have these problems?
Can I reproduce the problem in the MyFaces examples?

regards,

Martin


On 23 Jan 2005 22:06:33 -0000, svieujot@apache.org <sv...@apache.org> wrote:
> svieujot    2005/01/23 14:06:33
> 
>   Modified:    src/jsfapi/javax/faces/component UISelectMany.java
>                src/share/org/apache/myfaces/renderkit/html
>                         HtmlRendererUtils.java
>   Log:
>   Bugfix :  When no value were submitted, default values were set.
>   This caused a bug when the component wasn't displayed (for example if it was in a TabPanel's Tab that wasn't displayed).
>   It was reseting the backend bean's value, and sometime causing Null Pointer Exceptions.
> 
>   Revision  Changes    Path
>   1.15      +7 -3      incubator-myfaces/src/jsfapi/javax/faces/component/UISelectMany.java
> 
>   Index: UISelectMany.java
>   ===================================================================
>   RCS file: /home/cvs/incubator-myfaces/src/jsfapi/javax/faces/component/UISelectMany.java,v
>   retrieving revision 1.14
>   retrieving revision 1.15
>   diff -u -r1.14 -r1.15
>   --- UISelectMany.java 22 Jan 2005 16:47:17 -0000      1.14
>   +++ UISelectMany.java 23 Jan 2005 22:06:33 -0000      1.15
>   @@ -30,6 +30,11 @@
>     * @author Manfred Geiler (latest modification by $Author$)
>     * @version $Revision$ $Date$
>     * $Log$
>   + * Revision 1.15  2005/01/23 22:06:33  svieujot
>   + * Bugfix :  When no value were submitted, default values were set.
>   + * This caused a bug when the component wasn't displayed (for example if it was in a TabPanel's Tab that wasn't displayed).
>   + * It was reseting the backend bean's value, and sometime causing Null Pointer Exceptions.
>   + *
>     * Revision 1.14  2005/01/22 16:47:17  mmarinschek
>     * fixing bug with validation not called if the submitted value is empty; an empty string is submitted instead if the component is enabled.
>     *
>   @@ -252,7 +257,8 @@
> 
>            if(submittedValue instanceof String && ((String) submittedValue).length()==0)
>            {
>   -            submittedValue = null;
>   +            submittedValue = null; // TODO : This is a bug, if set to null, you'll get the following error :
>   +            // java.lang.NullPointerException at org.apache.myfaces.renderkit._SharedRendererUtils.getConvertedUISelectManyValue(_SharedRendererUtils.java:118)
>            }
> 
>            Object convertedValue = getConvertedValue(context, submittedValue);
> 
>   1.25      +18 -6     incubator-myfaces/src/share/org/apache/myfaces/renderkit/html/HtmlRendererUtils.java
> 
>   Index: HtmlRendererUtils.java
>   ===================================================================
>   RCS file: /home/cvs/incubator-myfaces/src/share/org/apache/myfaces/renderkit/html/HtmlRendererUtils.java,v
>   retrieving revision 1.24
>   retrieving revision 1.25
>   diff -u -r1.24 -r1.25
>   --- HtmlRendererUtils.java    22 Jan 2005 16:47:17 -0000      1.24
>   +++ HtmlRendererUtils.java    23 Jan 2005 22:06:33 -0000      1.25
>   @@ -139,7 +139,10 @@
>                //if the component has not been disabled
>                if(!isDisabledOrReadOnly(component))
>                {
>   -                ((EditableValueHolder) component).setSubmittedValue(RendererUtils.EMPTY_STRING);
>   +                ((EditableValueHolder) component).setSubmittedValue( null );
>   +                // Was .setSubmittedValue(RendererUtils.EMPTY_STRING) before.
>   +                // This caused a bug when the component wasn't displayed
>   +                // (for example if it was in a TabPanel's Tab that wasn't displayed).
>                }
>            }
>        }
>   @@ -177,7 +180,10 @@
>                //if the component has not been disabled
>                if(!isDisabledOrReadOnly(component))
>                {
>   -                ((EditableValueHolder) component).setSubmittedValue(Boolean.FALSE);
>   +                ((EditableValueHolder) component).setSubmittedValue( null );
>   +                // Was .setSubmittedValue(Boolean.FALSE) before.
>   +                // This caused a bug when the component wasn't displayed
>   +                // (for example if it was in a TabPanel's Tab that wasn't displayed).
>                }
>            }
>        }
>   @@ -220,7 +226,10 @@
>                //if the component has not been disabled
>                if(!isDisabledOrReadOnly(component))
>                {
>   -                ((EditableValueHolder) component).setSubmittedValue(RendererUtils.EMPTY_STRING);
>   +                ((EditableValueHolder) component).setSubmittedValue( null );
>   +                // Was .setSubmittedValue(RendererUtils.EMPTY_STRING) before.
>   +                // This caused a bug when the component wasn't displayed
>   +                // (for example if it was in a TabPanel's Tab that wasn't displayed).
>                }
>            }
>        }
>   @@ -251,7 +260,10 @@
> 
>                if(!isDisabledOrReadOnly(component))
>                {
>   -                ((EditableValueHolder) component).setSubmittedValue(RendererUtils.EMPTY_STRING);
>   +                ((EditableValueHolder) component).setSubmittedValue( null );
>   +                // Was .setSubmittedValue(RendererUtils.EMPTY_STRING) before.
>   +                // This caused a bug when the component wasn't displayed
>   +                // (for example if it was in a TabPanel's Tab that wasn't displayed).
>                }
>            }
>        }
> 
>

Re: cvs commit: incubator-myfaces/src/share/org/apache/myfaces/renderkit/html HtmlRendererUtils.java

Posted by Martin Marinschek <ma...@gmail.com>.
I have tried to fix the problem - can you check out the new version
and try it again?

regards,

Martin


On Mon, 24 Jan 2005 12:26:28 +0100, Martin Marinschek
<ma...@gmail.com> wrote:
> No, that is the same as before - if you look at the call to the
> isRendered() method a few lines before your suggested change...
> 
> if (
> /*here*/
>        !isRendered()
> /*here*/
> ) return;
>         for (Iterator it = getFacetsAndChildren(); it.hasNext(); )
>         {
>             UIComponent childOrFacet = (UIComponent)it.next();
>             if( childOrFacet.isRendered() ){
>             childOrFacet.processDecodes(context);
>             }
> 
> The problem is the following: if a component does not have a value
> (e.g. a listbox where no item is selected) the validator is not
> triggered - the RI solves that by providing an empty string for all
> components which are decoded but not disabled; and in discussions with
> Manfred we decided that this possibilty would be best to take in our
> case, too.
> 
> Now it has to be made sure that only the components which are encoded
> are decoded, and I don't know where the encoding is triggered for the
> tabbed-pane - exactly there the decoding should be restricted to the
> rendered components.
> 
> regards,
> 
> Martin
> 
> 
> On Mon, 24 Jan 2005 07:08:03 -0400, Sylvain Vieujot <sv...@freelance.com> wrote:
> >  Here is a fix I've found about this problem, but I'm not 100% sure about
> > it.
> >  Could you give me your opinion ?
> >
> >  The suggested fix in in bold red.
> >  In UIComponentBase (jsfapi/javax/faces/component/UIComponentBase.java) :
> >
> >      public void processDecodes(FacesContext context)
> >      {
> >          if (context == null) throw new NullPointerException("context");
> >          if (!isRendered()) return;
> >          for (Iterator it = getFacetsAndChildren(); it.hasNext(); )
> >          {
> >              UIComponent childOrFacet = (UIComponent)it.next();
> >              if( childOrFacet.isRendered() ){
> >              childOrFacet.processDecodes(context);
> >              }
> >          }
> >          try
> >          {
> >              decode(context);
> >          }
> >          catch (RuntimeException e)
> >          {
> >              context.renderResponse();
> >              throw e;
> >          }
> >      }
> >
> >  Thanks for your help.
> >
> >
> >  Sylvain.
> >
> >  On Mon, 2005-01-24 at 10:21 +0100, Martin Marinschek wrote:
> >  The thing is that all components which are not rendered shouldn't be
> > decoded - so it is the problem of the tabbed panel to decode only those
> > children which are rendered to the client. We need to have those two lines
> > in the code due to several reasons - please reread the threads I have
> > exchanged with Heath and Sean on this topic, the tabbed panel needs to be
> > changed to decode only those components which are rendered; this would be
> > the correct fix. regards, Martin On Sun, 23 Jan 2005 22:24:50 -0400, Sylvain
> > Vieujot <sv...@freelance.com> wrote: > Hello Martin, > > In fact, I think the
> > all isDisabledOrReadOnly block should be removed. > The problem is with any
> > component that uses those methodes. > Among which : inputText,
> > inputTextArea, list box, combo box. > In fact it worked before - I mean a
> > few weeks ago - but with the new code, > I think some changes have been made
> > in the validation or convertion process > that uncovers this bug. > I saw 2
> > symptoms, all using the tab panel. > Indeed, when you have 2 tabs, some
> > components aren't rendered. > So, the submitted value is null. But the
> > previous code replaced the non > submitted value with a default value (a
> > false, or an empty string). > This caused the unrendered UISelectMany
> > components to generate a null > pointer exception, and for the unrendered
> > text/textarea components, this > caused the backing bean values to be reset
> > to the empty string. > I guess the checkbox & radio buttons would have had
> > problems too, and I > guess we could have had the same bug with a component
> > that had > rendered="false" or disabled="true" (but I didn't test those
> > cases). > > So, I think when no value has been submitted, we shouldn't set
> > any > submitted value, and if you agree, we should remove the
> > isDisabledOrReadOnly > blocks all together in the patch. > Do you see any
> > reason why we submitted artificial values in the first place > ? > > Best
> > regards, > > Sylvain > > > On Mon, 2005-01-24 at 01:09 +0100, Martin
> > Marinschek wrote: > Hi Sylvain, I tried to resolve a problem with validation
> > not happening in > some cases with that lines - so I believe these lines are
> > essentially > correct. The problem lies in the fact that obviously something
> > is decoded > which shouldn't be decoded (and I mean that the parent of the
> > component is > calling the decode on the component even though it should not
> > cause the > component is not visible). Can you tell me again with which
> > component you > have these problems? Can I reproduce the problem in the
> > MyFaces examples? > regards, Martin On 23 Jan 2005 22:06:33 -0000,
> > svieujot@apache.org > <sv...@apache.org> wrote: > svieujot 2005/01/23
> > 14:06:33 > > Modified: > src/jsfapi/javax/faces/component UISelectMany.java
> > > > src/share/org/apache/myfaces/renderkit/html > HtmlRendererUtils.java >
> > Log: > > Bugfix : When no value were submitted, default values were set. >
> > This > caused a bug when the component wasn't displayed (for example if it
> > was in a > TabPanel's Tab that wasn't displayed). > It was reseting the
> > backend bean's > value, and sometime causing Null Pointer Exceptions. > >
> > Revision Changes > Path > 1.15 +7 -3 >
> > incubator-myfaces/src/jsfapi/javax/faces/component/UISelectMany.java > > >
> > Index: UISelectMany.java > >
> > =================================================================== > RCS >
> > file: >
> > /home/cvs/incubator-myfaces/src/jsfapi/javax/faces/component/UISelectMany.java,v
> > > > retrieving revision 1.14 > retrieving revision 1.15 > diff -u -r1.14 >
> > -r1.15 > --- UISelectMany.java 22 Jan 2005 16:47:17 -0000 1.14 > +++ >
> > UISelectMany.java 23 Jan 2005 22:06:33 -0000 1.15 > @@ -30,6 +30,11 @@ > * >
> > @author Manfred Geiler (latest modification by $Author$) > * @version >
> > $Revision$ $Date$ > * $Log$ > + * Revision 1.15 2005/01/23 22:06:33 svieujot
> > > > + * Bugfix : When no value were submitted, default values were set. > +
> > * > This caused a bug when the component wasn't displayed (for example if it
> > was > in a TabPanel's Tab that wasn't displayed). > + * It was reseting the
> > > backend bean's value, and sometime causing Null Pointer Exceptions. > + *
> > > > * Revision 1.14 2005/01/22 16:47:17 mmarinschek > * fixing bug with >
> > validation not called if the submitted value is empty; an empty string is >
> > submitted instead if the component is enabled. > * > @@ -252,7 +257,8 @@ > >
> > > if(submittedValue instanceof String && ((String) >
> > submittedValue).length()==0) > { > - submittedValue = null; > + >
> > submittedValue = null; // TODO : This is a bug, if set to null, you'll get >
> > the following error : > + // java.lang.NullPointerException at >
> > org.apache.myfaces.renderkit._SharedRendererUtils.getConvertedUISelectManyValue(_SharedRendererUtils.java:118)
> > > > } > > Object convertedValue = getConvertedValue(context,
> > submittedValue); > > > 1.25 +18 -6 >
> > incubator-myfaces/src/share/org/apache/myfaces/renderkit/html/HtmlRendererUtils.java
> > > > > Index: HtmlRendererUtils.java > >
> > =================================================================== > RCS >
> > file: >
> > /home/cvs/incubator-myfaces/src/share/org/apache/myfaces/renderkit/html/HtmlRendererUtils.java,v
> > > > retrieving revision 1.24 > retrieving revision 1.25 > diff -u -r1.24 >
> > -r1.25 > --- HtmlRendererUtils.java 22 Jan 2005 16:47:17 -0000 1.24 > +++ >
> > HtmlRendererUtils.java 23 Jan 2005 22:06:33 -0000 1.25 > @@ -139,7 +139,10 >
> > @@ > //if the component has not been disabled > >
> > if(!isDisabledOrReadOnly(component)) > { > - ((EditableValueHolder) >
> > component).setSubmittedValue(RendererUtils.EMPTY_STRING); > + >
> > ((EditableValueHolder) component).setSubmittedValue( null ); > + // Was >
> > .setSubmittedValue(RendererUtils.EMPTY_STRING) before. > + // This caused a
> > > bug when the component wasn't displayed > + // (for example if it was in a
> > > TabPanel's Tab that wasn't displayed). > } > } > } > @@ -177,7 +180,10 @@
> > > > //if the component has not been disabled > >
> > if(!isDisabledOrReadOnly(component)) > { > - ((EditableValueHolder) >
> > component).setSubmittedValue(Boolean.FALSE); > + ((EditableValueHolder) >
> > component).setSubmittedValue( null ); > + // Was >
> > .setSubmittedValue(Boolean.FALSE) before. > + // This caused a bug when the
> > > component wasn't displayed > + // (for example if it was in a TabPanel's
> > Tab > that wasn't displayed). > } > } > } > @@ -220,7 +226,10 @@ > //if the
> > > component has not been disabled > if(!isDisabledOrReadOnly(component)) > {
> > > > - ((EditableValueHolder) >
> > component).setSubmittedValue(RendererUtils.EMPTY_STRING); > + >
> > ((EditableValueHolder) component).setSubmittedValue( null ); > + // Was >
> > .setSubmittedValue(RendererUtils.EMPTY_STRING) before. > + // This caused a
> > > bug when the component wasn't displayed > + // (for example if it was in a
> > > TabPanel's Tab that wasn't displayed). > } > } > } > @@ -251,7 +260,10 @@
> > > > > if(!isDisabledOrReadOnly(component)) > { > - ((EditableValueHolder) >
> > component).setSubmittedValue(RendererUtils.EMPTY_STRING); > + >
> > ((EditableValueHolder) component).setSubmittedValue( null ); > + // Was >
> > .setSubmittedValue(RendererUtils.EMPTY_STRING) before. > + // This caused a
> > > bug when the component wasn't displayed > + // (for example if it was in a
> > > TabPanel's Tab that wasn't displayed). > } > } > } > >
>