----- Original Message -----
From: "Matt Read" <[EMAIL PROTECTED]>
To: "Struts Developers List" <[EMAIL PROTECTED]>
Sent: Wednesday, March 06, 2002 2:07 PM
Subject: RE: Re[2]: cvs commit:
jakarta-struts/src/share/org/apache/struts/taglib/bean WriteTag.java


> Don't worry about the time, my bodyclock doesn't know which end is up
> mostly.
>
> Unfortunately the getter and setter for the format attribute are used in
> cases like this:
> <bean:write name="document" property="createDate" format="d MMM ''yy" />

Doh! I missed the fact that the 'format' attribute uses the 'formatStr'
variable (i.e. the names don't match).

>
> The behaviour as the code stands in CVS means that the format attribute
> overrides the formatKey attribute if they're both used which is fair
enough
> I suppose. Obviously the problem still stands where if neither are
specified
> then the previous value of formatStr will be used so I guess the solution
is
> as per your original suggestion, patch below if you want to check it in.

This won't work - it was based on the misunderstanding I had that you
cleared up above. The original attribute values need to be preserved so that
the tag handlers can be reused successfully in containers that do that.

I'm going to make a change so that a local variable is used within
formatValue(), instead of formatStr being used directly. That will avoid the
whole issue.

--
Martin Cooper


>
> diff -w -r1.18 WriteTag.java
> 414c414,415
> <         if( format!=null )
> ---
> >         if( format!=null ) {
> >             formatStr = null;
> 416c417
> <         else
> ---
> >         } else
>
>
> -----Original Message-----
> From: Martin Cooper [mailto:[EMAIL PROTECTED]]
> Sent: 06 March 2002 21:37
> To: Struts Developers List
> Subject: Re: Re[2]: cvs commit:
> jakarta-struts/src/share/org/apache/struts/taglib/bean WriteTag.java
>
>
> The problem with that alone is that it will lead to the same problem
further
> down. For example, if formatKey is null, and value is a Number, the next
> test will be to check formatStr against null. That could once again be
> checking against a value that was left over from a previous instance of
the
> tag.
>
> It seems to me that we should just make formatStr local to formatValue()
and
> be done with it. If you have a chance (it must be getting a bit late over
> there now!), could you give that a quick try, and let me know the result?
I
> can check in the change.
>
> --
> Martin Cooper
>
>
> ----- Original Message -----
> From: "Matt Read" <[EMAIL PROTECTED]>
> To: "Struts Developers List" <[EMAIL PROTECTED]>
> Sent: Wednesday, March 06, 2002 1:06 PM
> Subject: RE: Re[2]: cvs commit:
> jakarta-struts/src/share/org/apache/struts/taglib/bean WriteTag.java
>
>
> > The fix I had made is as follows. Is there any reason why one would be
> > better than the other? I'm new to editing struts source and would
> appreciate
> > pointers.
> >
> > Thanks,
> > Matt.
> >
> > diff -r1.18 WriteTag.java
> > 346c346
> > <             if( ( formatStr==null ) && ( formatKey!=null ) ) {
> > ---
> > >             if( formatKey!=null ) {
> >
> > -----Original Message-----
> > From: Martin Cooper [mailto:[EMAIL PROTECTED]]
> > Sent: 06 March 2002 20:37
> > To: Struts Developers List; Oleg V Alexeev
> > Subject: Re: Re[2]: cvs commit:
> > jakarta-struts/src/share/org/apache/struts/taglib/bean WriteTag.java
> >
> >
> > The container is not required to call release() between uses of a pooled
> tag
> > handler. Resin, which Matt is using, doesn't call release() between
uses,
> > which is why I questioned the fix.
> >
> > From a quick look at the code, I believe the problem is that formatStr
> > (which is not an attribute, although it is defined as if it is, with
> getter
> > and setter) is tested against null, and assigned within formatValue(). I
> > don't know why it is tested for null, since there is no other method
that
> > manipulates this variable.
> >
> > I think the bug is that formatStr keeps its value from one use of the
tag
> to
> > the next, and the first usage of that variable is a test against null.
> After
> > the first use, it will most likely be non-null in subsequent uses, thus
> > causing the problem. The simplest fix is probably to null out formatStr
at
> > the end of formatValue(), so that the entry state is the same for the
> first
> > and subsequent uses of the tag. Let me know if you'd like me to make
that
> > change, Oleg.
> >
> > --
> > Martin Cooper
> >
> >
> > ----- Original Message -----
> > From: "Oleg V Alexeev" <[EMAIL PROTECTED]>
> > To: "Struts Developers List" <[EMAIL PROTECTED]>
> > Sent: Wednesday, March 06, 2002 11:55 AM
> > Subject: Re[2]: cvs commit:
> > jakarta-struts/src/share/org/apache/struts/taglib/bean WriteTag.java
> >
> >
> > > Hello Martin,
> > >
> > > I think that such error can be thrown in containers with tag pooling -
> > > so prevously used tag instance will have default attribute value.
> > >
> > > Wednesday, March 06, 2002, 10:29:06 PM, you wrote:
> > >
> > > MC> Interesting. This should not make a difference unless there is
> another
> > bug
> > > MC> involved (either in the tag or the container). What were the
> symptoms
> > > MC> involved here, and which container was being used?
> > >
> > > MC> --
> > > MC> Martin Cooper
> > >
> > >
> > > MC> ----- Original Message -----
> > > MC> From: <[EMAIL PROTECTED]>
> > > MC> To: <[EMAIL PROTECTED]>
> > > MC> Sent: Wednesday, March 06, 2002 1:11 AM
> > > MC> Subject: cvs commit:
> > jakarta-struts/src/share/org/apache/struts/taglib/bean
> > > MC> WriteTag.java
> > >
> > >
> > > >> oalexeev    02/03/06 01:11:10
> > > >>
> > > >>   Modified:    src/share/org/apache/struts/taglib/bean
WriteTag.java
> > > >>   Log:
> > > >>   Fix - formatKey class variable is not set to null in release()
> > method.
> > > >>   Bug submitted by "Matt Read" <[EMAIL PROTECTED]>
> > > >>
> > > >>   Revision  Changes    Path
> > > >>   1.18      +5 -4
> > > MC>
jakarta-struts/src/share/org/apache/struts/taglib/bean/WriteTag.java
> > > >>
> > > >>   Index: WriteTag.java
> > > >>
===================================================================
> > > >>   RCS file:
> > > MC>
> >
>
/home/cvs/jakarta-struts/src/share/org/apache/struts/taglib/bean/WriteTag.ja
> > > MC> va,v
> > > >>   retrieving revision 1.17
> > > >>   retrieving revision 1.18
> > > >>   diff -u -r1.17 -r1.18
> > > >>   --- WriteTag.java 17 Jan 2002 18:20:16 -0000 1.17
> > > >>   +++ WriteTag.java 6 Mar 2002 09:11:10 -0000 1.18
> > > >>   @@ -1,7 +1,7 @@
> > > >>    /*
> > > >>   - * $Header:
> > > MC>
> >
>
/home/cvs/jakarta-struts/src/share/org/apache/struts/taglib/bean/WriteTag.ja
> > > MC> va,v 1.17 2002/01/17 18:20:16 oalexeev Exp $
> > > >>   - * $Revision: 1.17 $
> > > >>   - * $Date: 2002/01/17 18:20:16 $
> > > >>   + * $Header:
> > > MC>
> >
>
/home/cvs/jakarta-struts/src/share/org/apache/struts/taglib/bean/WriteTag.ja
> > > MC> va,v 1.18 2002/03/06 09:11:10 oalexeev Exp $
> > > >>   + * $Revision: 1.18 $
> > > >>   + * $Date: 2002/03/06 09:11:10 $
> > > >>     *
> > > >>     *
> > ====================================================================
> > > >>     *
> > > >>   @@ -89,7 +89,7 @@
> > > >>     * output stream, optionally filtering characters that are
> sensitive
> > in
> > > MC> HTML.
> > > >>     *
> > > >>     * @author Craig R. McClanahan
> > > >>   - * @version $Revision: 1.17 $ $Date: 2002/01/17 18:20:16 $
> > > >>   + * @version $Revision: 1.18 $ $Date: 2002/03/06 09:11:10 $
> > > >>     */
> > > >>
> > > >>    public class WriteTag extends TagSupport {
> > > >>   @@ -430,6 +430,7 @@
> > > >>            property = null;
> > > >>            scope = null;
> > > >>            formatStr = null;
> > > >>   +        formatKey = null;
> > > >>            localeKey = null;
> > > >>            bundle = null;
> > > >>
> > > >>
> > > >>
> > > >>
> > > >>
> > > >> --
> > > >> To unsubscribe, e-mail:
> > > MC> <mailto:[EMAIL PROTECTED]>
> > > >> For additional commands, e-mail:
> > > MC> <mailto:[EMAIL PROTECTED]>
> > > >>
> > >
> > >
> > > MC> --
> > > MC> To unsubscribe, e-mail:
> > <mailto:[EMAIL PROTECTED]>
> > > MC> For additional commands, e-mail:
> > <mailto:[EMAIL PROTECTED]>
> > >
> > >
> > >
> > > --
> > > Best regards,
> > >  Oleg                            mailto:[EMAIL PROTECTED]
> > >
> > >
> > >
> > > --
> > > To unsubscribe, e-mail:
> > <mailto:[EMAIL PROTECTED]>
> > > For additional commands, e-mail:
> > <mailto:[EMAIL PROTECTED]>
> > >
> >
> >
> > --
> > To unsubscribe, e-mail:
> <mailto:[EMAIL PROTECTED]>
> > For additional commands, e-mail:
> <mailto:[EMAIL PROTECTED]>
> >
> >
> > --
> > To unsubscribe, e-mail:
> <mailto:[EMAIL PROTECTED]>
> > For additional commands, e-mail:
> <mailto:[EMAIL PROTECTED]>
> >
>
>
> --
> To unsubscribe, e-mail:
<mailto:[EMAIL PROTECTED]>
> For additional commands, e-mail:
<mailto:[EMAIL PROTECTED]>
>
>
> --
> To unsubscribe, e-mail:
<mailto:[EMAIL PROTECTED]>
> For additional commands, e-mail:
<mailto:[EMAIL PROTECTED]>
>


--
To unsubscribe, e-mail:   <mailto:[EMAIL PROTECTED]>
For additional commands, e-mail: <mailto:[EMAIL PROTECTED]>

Reply via email to