On 7/6/07, Paul Benedict <[EMAIL PROTECTED]> wrote:
The ticket issue shows the conversation starting without a form and then growing to include the form. So I believe it was appropriate to address both situations, which I think are okay.
I had missed that in the comments :(
First, in all the years of development of Struts, I have never come across one user mail or program code where people are asking to retrieve values on cancellation. Please find one for me so I can be corrected. In fact, I can't think of one framework which allows this anyway. I put this in the "bad practice" department and believe this should not be supported in anyway -- and the current situation is an oversight. The action is canceled, the form is part of the action, and so the form has no job which is very much implied by the canceling of validation. Now granted, there seems to be concern here with you and Martin. However, I think this is a justified change, and going from one version to the next does not have to be 100% backwards compatible .. just about 98 or 99%, in my opinion. Remember, this codebase has no 2.0 version -- that's done by the WW port -- so if things are going to move forward, and perhaps dream to fix some rather odd use cases, I find this appropriate just as much as Struts 2.1 alters some functionality of Struts 2.0.
IMO people who want to move forward should use Struts2 - Struts 1 shouldn't break compatibilty lightly - I see no great benefit to changing how this has worked for 6 years and just pain for anyone who has happened to rely on it. Also with the Command Chain introduced in Struts 1.3 its possible to easily provide both new functionality and compatibility with different Command implementations. So I'm still against this change as it is now. Niall
Paul Niall Pemberton wrote: > I don't have a problem with setting the "cancellation" attribute where > there is no form - but this changes goes further than whats mentioned > in STR-1674 with the form no longer being populated when "cancelled". > This breaks compatibility with previous Struts versions and I'm > against it. > > Niall > > On 7/2/07, [EMAIL PROTECTED] <[EMAIL PROTECTED]> wrote: >> Author: pbenedict >> Date: Sun Jul 1 21:03:17 2007 >> New Revision: 552396 >> >> URL: http://svn.apache.org/viewvc?view=rev&rev=552396 >> Log: >> STR-1674: Check cancellation first and possibly skip population >> >> Modified: >> >> struts/struts1/trunk/core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java >> >> >> Modified: >> struts/struts1/trunk/core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java >> >> URL: >> http://svn.apache.org/viewvc/struts/struts1/trunk/core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java?view=diff&rev=552396&r1=552395&r2=552396 >> >> ============================================================================== >> >> --- >> struts/struts1/trunk/core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java >> (original) >> +++ >> struts/struts1/trunk/core/src/main/java/org/apache/struts/chain/commands/AbstractPopulateActionForm.java >> Sun Jul 1 21:03:17 2007 >> @@ -45,21 +45,26 @@ >> */ >> public boolean execute(ActionContext actionCtx) >> throws Exception { >> - // Is there a form bean for this request? >> + >> + ActionConfig actionConfig = actionCtx.getActionConfig(); >> ActionForm actionForm = actionCtx.getActionForm(); >> >> + // First determine if the request was cancelled >> + handleCancel(actionCtx, actionConfig, actionForm); >> + >> + // Is there a form bean for this request? >> if (actionForm == null) { >> return (false); >> } >> >> - // Reset the form bean property values >> - ActionConfig actionConfig = actionCtx.getActionConfig(); >> + // If request is cancelled, form manipulation is prevented >> + if (actionCtx.getCancelled().booleanValue()) { >> + return (false); >> + } >> >> + // Reset and repopulate the form bean property values >> reset(actionCtx, actionConfig, actionForm); >> - >> populate(actionCtx, actionConfig, actionForm); >> - >> - handleCancel(actionCtx, actionConfig, actionForm); >> >> return (false); >> }
--------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
