Re: [Webwork-devel] Webwork 1.0.3 / CVS taglib totally broken?

SourceForge Headquarters 225 Broadway Suite 1600 San Diego, CA 92101 +1 (858) 422-6466

I agree that reset logic should be moved to doEndTag() but this is different
from null'ing attributes. Null'ing attributes should still be in release().
I don't want to make the blanket change at this time. I did fix Property and
Action. I would prefer to make a couple of fixes and all containers should
work. I changed the escaping of outer quotes to true by default. The reason
it was false was just a first guess. I guessed wrong :). I haven't tested
these changes on Resin yet. I can't get to caucho but I will by early this
week.

-Matt

----- Original Message -----
From: "Mike Cannon-Brookes" <mi...@at...>
To: <ma...@sm...>
Cc: <web...@li...>
Sent: Monday, May 27, 2002 2:50 AM
Subject: Re: [Webwork-devel] Webwork 1.0.3 / CVS taglib totally broken?

> Ok - I've checked and from preliminary looks it seems like the
ComponentTag
> is fixed in Orion (haven't tested Resin yet - will do)
>
> PropertyTag:
> I had to fix the PropertyTag again though.
>
> Simple fix: (can't check in myself but here's the details)
> - remove return statement that's not at the end of the doEndTag method)
> - call release() manually before return EVAL_PAGE;
> (see attachment)
>
> Again - the problem here is that data is reset in the release() method
which
> is incorrect ;)
>
> I have a suggested fix for every tag - rename ALL the release() methods to
> reset() and call them manually at the end of the doEndTag() methods (or
> before every return EVAL_PAGE statement) as well. This keeps the 'reset'
> logic compartmentalised but at the same time makes sure that it will be
> called on ALL containers.
>
> I've done this in the attached file so you can see what I mean.
>
> TextUtil:
> Also I changed the default value of "escapeOuterQuotes" to true. Why would
> you ever want this to be false? (Having it false by default means for
> example that any formfield with "text" as a value only gets text which is
v.
> dangerous) Can we change this in CVS? Why was escapeOuterQuotes ever
needed?
>
>
> I'll yell again if I find any more specific problems - but I can see that
> the release() method does things in other tag (which I probably don't use)
> which is bad.
>
> Cheers,
> Mike
>
> PS More investigating (this time in Resin) since I wrote the above - there
> are now a lot of NPEs due to the new ValueStack stuff. Line 96 of
> ValueStack.java and line 403 of AbstractValueStack.java (both probably
> related to null values being looked up and not checked for before calling
> method on the value object)
>
>
> Mike Cannon-Brookes
> mi...@at...
>
> ATLASSIAN - Your J2EE Expert Partner
> --------------------------------------------------------
> > Brilliant Software - http://www.atlassian.com/software
> > Legendary Services - http://www.atlassian.com/support
>
>
> On 24/5/02 1:03 AM, "ma...@sm..." (ma...@sm...) penned the
> words:
>
> > Alright. Thanks for the reply. We'll get this worked
> > out and commented accordingly. Whatever the outcome,
> > I'll check it against Resin, WL, and Tomcat. I'll leave
> > you and others to check it against Orion. As soon as I
> > get a thumbs up from the community, I'll release a
> > patch 1.0.4.
> >
> > -Matt
> >
> > On Fri, 24 May 2002, Mike Cannon-Brookes wrote
> >
> >>
> >> Matt,
> >>
> >> Thanks mate - I'll check the CVS in the morning and
> > report back. I found a
> >> bunch of changes I needed to make today to get it
> > working, so I'll see how
> >> CVS works for me and start from there again tomorrow
> > (with more mental
> >> energy).
> >>
> >> Re: release() - this is NOT the preferred method at
> > all ;)
> >>
> >> Again (yes, I've checked now with the spec and the
> > author of JSP Tags book)
> >> release() is called by the container to release
> > resources associated with
> >> the tag. It can be called at ANY time the container
> > wishes (you have no
> >> guarantee of when it is called). The only restriction
> > is that it must be
> >> called before the tag instance is garbage collected.
> >>
> >> So - what you have is lazy programmers (WL, Tomcat
> > are the two glaring
> >> examples) who just call release() after every tag
> > invocation - which leads
> >> to people's misplaced belief that it MUST be called
> > everytime. This is not
> >> so! :) Fast containers (like Resin and Orion) pool
> > tag instances and reuse
> >> them (maximising speed), and call release() as few
> > times as possible
> >> (sometimes never). I'm sure it's harder to code it
> > this way, but the end
> >> result is speed.
> >>
> >> Net result:
> >>
> >>    NEVER do anything you want to happen in release()!
> >>
> >> This is true across all the JSP tags in WW and other
> > projects. If you want
> >> logic to occur, or variables to be reset for each tag
> > invocation, reset them
> >> at the end of the doEndTag() method (either before
> > returning, or as a
> >> finally block).
> >>
> >> I'll check CVS in the morning - good work on the
> > quick fixes though!
> >>
> >> Cheers,
> >> Mike
> >>
> >> On 23/5/02 10:20 PM, "ma...@sm..."
> > (ma...@sm...) penned the
> >> words:
> >>
> >>> For WebLogic, using release is the preferred
> > method. I
> >>> disagree with your blanket statement. I fixed the
> >>> release order in the latest CVS and in the branch.
> > I'm
> >>> told that it is working correctly on Orion but I'm
> >>> waiting for official confirmation. I would
> > appreciate
> >>> if you could check it with the latest CVS and let me
> >>> know.
> >>>
> >>> On Thu, 23 May 2002, Mike Cannon-Brookes wrote
> >>>
> >>>>
> >>>> Yes - release() is the bane of tag developers.
> >>>>
> >>>> The simple rule is basically - if you're using
> >>> release, you're doing things
> >>>> wrong!
> >>>>
> >>>> -mike
> >>>>
> >>>> On 23/5/02 4:28 PM, "Heng Sin Low"
> >>> (low...@ya...) penned the words:
> >>>>
> >>>>> Ha, finally u got what I mean :)
> >>>>>
> >>>>> Yes, doEndTag should be the right place. I
> > remember
> >>>>> Matt have done a massive check in recently that
> > move
> >>>>> all this resetting stuff to the release() method.
> >>>>>
> >>>>> Regards,
> >>>>> Low
> >>>>>
> >>>>>> I'm trying to what out what changes have occurred
> >>>>> recently, but the taglib
> >>>>> in CVS is currently broken and not backward
> >>>>> compatible.
> >>>>>
> >>>>>
> >>>>> __________________________________________________
> >>>>> Do You Yahoo!?
> >>>>> LAUNCH - Your Yahoo! Music Experience
> >>>>> http://launch.yahoo.com
> >>>>>
> >>>>>
> >>>
> > _______________________________________________________________
> >>>>>
> >>>>> Don't miss the 2002 Sprint PCS Application
> >>> Developer's Conference
> >>>>> August 25-28 in Las Vegas --
> >>> http://devcon.sprintpcs.com/adp/index.cfm
> >>>>>
> >>>>> _______________________________________________
> >>>>> Webwork-devel mailing list
> >>>>> Web...@li...
> >>>>>
> >>>
> > https://lists.sourceforge.net/lists/listinfo/webwork-devel
> >>>>
> >>>>
> >>>>
> >>>
> > _______________________________________________________________
> >>>>
> >>>> Don't miss the 2002 Sprint PCS Application
> >>> Developer's Conference
> >>>> August 25-28 in Las Vegas --
> >>> http://devcon.sprintpcs.com/adp/index.cfm
> >>>>
> >>>> _______________________________________________
> >>>> Webwork-devel mailing list
> >>>> Web...@li...
> >>>>
> >>>
> > https://lists.sourceforge.net/lists/listinfo/webwork-devel
> >>>
> >>>
> >>>
> >>>
> > _______________________________________________________________
> >>>
> >>> Don't miss the 2002 Sprint PCS Application
> > Developer's Conference
> >>> August 25-28 in Las Vegas --
> > http://devcon.sprintpcs.com/adp/index.cfm
> >>>
> >>> _______________________________________________
> >>> Webwork-devel mailing list
> >>> Web...@li...
> >>>
> > https://lists.sourceforge.net/lists/listinfo/webwork-devel
> >
> >
> >
> > _______________________________________________________________
> >
> > Don't miss the 2002 Sprint PCS Application Developer's Conference
> > August 25-28 in Las Vegas -- http://devcon.sprintpcs.com/adp/index.cfm
> >
> > _______________________________________________
> > Webwork-devel mailing list
> > Web...@li...
> > https://lists.sourceforge.net/lists/listinfo/webwork-devel
>
>