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

[Mike Cannon-B. <mi...@at...>]

On 27/5/02 9:38 PM, "Matt Baldree" (ma...@sm...) penned the words:

> 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 don't understand. Every webwork tag (that I can see) assumes that
attributes are null at the start of doStartTag(). Moving everything into
reset() and calling from doEndTag() and release() fixes this problem for all
containers!

Why would you not want to make this change? :)

> 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.

Cool - I'll update my CVS and try again today - another day battling Resin.
;)

Cheers,
Mike

> -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
>> 
>> 
> 
> 
> 
> _______________________________________________________________
> 
> 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