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

SourceForge Headquarters 1320 Columbia Street Suite 310 San Diego, CA 92101 +1 (858) 422-6466

I just looked at this again and the PropertyTag is _still_ broken I think.
Somehow I don't seem to be getting through here - my fix worked perfectly on
all containers, why was it not used?! :)

You still have valueAttr being set to null in the release() method.

However, property tag can be used like so <webwork:property /> but in this
case (as the valueAttr still has the value from the last time property was
used) the wrong value is used!

<ww:property value="foo">
    <ww:property value="../bar" />
    <ww:property /> <-- this should print use foo as the value correct? But
here it will use the 'cached' ../bar!
</ww:property>

Once again - everything should be reset in doEndTag! I cannot find a single
tag (browsing the ww source) that should EVER use release()!

-mike

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