You're not out of line at all - this list is the place to discuss enhancement requests and requests which come with a patch attached are more welcome than those without :-)
Having said that, I'm against this change. If the user disables javascript in their browser then the cancel button no longer works. Even with your enhanced patch to turn on/off the additional behaviour by setting a new javascript attribute to true/false, the fact remains that the cancel button would stop working with javascript disabled. I bet we would have questions every week titled "Cancel Button Doesn't Work" on the user list and it will get boring typing the same "Did you set the javascript='true' attribute etc etc." You might say that the same arguments is true for the existing javascript in the CancelTag (i.e. onclick="bCancel=true") but I wouldn't agree with that. Although that javascript would also not work with javascript disabled, the fact is that it interacts with the javascript rendered by the validator framework - that javascript IMO is an optional extra because with javascript disabled the validator framework's server side validation would still work and the system would not be *broken*. Another point about the issue you are trying to resolve is that its a browser/html isssue and nothing to do with how Struts works. If we put in this patch to implement some custom javascript behaviour then what about the next person who wants their own funky bit of javascript in Struts to implement their custom behaviour? If you look at the CancelTag it doesn't actually do much "Struts" wise - you could easily do what it does directly in html (including the behaviour you want), it just saves keystrokes. Maybe Jakarta Taglibs would be a better place for this tag and the behaviour that you want - they have an "input" tags component. http://jakarta.apache.org/taglibs/doc/input-doc/intro.html When Struts was *born* there wasn't much else around - but now many of the Struts tags can be replaced with JSTL and the current view (which I agree with) seems to be that the tags should be split into *core* Struts tags - tags which implement behaviour specific to Struts and the rest which will be kept on as "legacy" tags. The idea being that we should only continue to develop *core* tags. IMO the cancel tag falls into the later category - others may differ though. Niall ----- Original Message ----- From: "Marcus Breese" <[EMAIL PROTECTED]> To: <[EMAIL PROTECTED]> Sent: Tuesday, June 22, 2004 7:22 PM Subject: PATCH: html:cancel tag. Made a javascript version for submitting > I'm submitting this again here because my Bugzilla patch hasn't gone > anywhere, so I don't know if anyone likes this idea or not. > Basically, if the cancel button is after the submit button on a form, > and the user hits 'enter' to submit the form, the the cancel button is > the one that is activated, causing the form to be cancelled. So, I > added a "javascript" property to the cancel button, that when true > causes the javascript button to be rendered as a combination html > button and a hidden text field that are linked through a javascript > onclick event. It is completely backwards compatible and doesn't > break any current pages (that I know of). > > If I'm completely out of line sending this here, I appologize... > > Bugzilla link: http://issues.apache.org/bugzilla/show_bug.cgi?id=25860 > > PATCH > ==================== > > Index: doc/userGuide/struts-html.xml > =================================================================== > RCS file: /home/cvspublic/jakarta-struts/doc/userGuide/struts-html.xml,v > retrieving revision 1.67 > diff -u -r1.67 struts-html.xml > --- doc/userGuide/struts-html.xml 2 Jan 2004 11:55:38 -0000 1.67 > +++ doc/userGuide/struts-html.xml 3 Jan 2004 00:44:06 -0000 > @@ -410,6 +410,16 @@ > disabled. > </info> > </attribute> > + <attribute> > + <name>javascript</name> > + <required>false</required> > + <rtexprvalue>true</rtexprvalue> > + <info> > + Set to <code>true</code> if this input field should be > + rendered using a javascript button/hidden field combination > + as opposed to a submit button. > + </info> > + </attribute> > > <attribute> > <name>onblur</name> > Index: src/share/org/apache/struts/taglib/html/CancelTag.java > =================================================================== > RCS file: /home/cvspublic/jakarta-struts/src/share/org/apache/struts/taglib/html/Cance lTag.java,v > retrieving revision 1.13 > diff -u -r1.13 CancelTag.java > --- src/share/org/apache/struts/taglib/html/CancelTag.java 31 Jul 2003 > 00:19:04 -0000 1.13 > +++ src/share/org/apache/struts/taglib/html/CancelTag.java 3 Jan 2004 > 00:44:06 -0000 > @@ -106,6 +106,12 @@ > */ > protected String value = null; > > + /** > + * Should this be rendered using javascript ? > + */ > + > + protected String javascript = null; > + > > // ------------------------------------------------------------- Properties > > @@ -144,6 +150,35 @@ > } > > > + > + /** > + * return the javascript value > + */ > + public String getJavascript() { > + return javascript; > + } > + > + /** > + * @param javascript > + */ > + public void setJavascript(String javascript) { > + this.javascript = javascript; > + } > + > + /** > + * > + * Return a boolean for if this should be rendered using javascript > + */ > + > + public boolean renderJavascript() { > + if (javascript!=null && javascript.toUpperCase().equals("TRUE")) { > + return true; > + } > + return false; > + } > + > + > + > // --------------------------------------------------------- Public Methods > > > @@ -192,10 +227,14 @@ > > // Generate an HTML element > StringBuffer results = new StringBuffer(); > - results.append("<input type=\"submit\""); > - results.append(" name=\""); > - results.append(property); > - results.append("\""); > + if (renderJavascript()) { > + results.append("<input type=\"button\""); > + } else { > + results.append("<input type=\"submit\""); > + results.append(" name=\""); > + results.append(property); > + results.append("\""); > + } > if (accesskey != null) { > results.append(" accesskey=\""); > results.append(accesskey); > @@ -212,12 +251,33 @@ > results.append(prepareEventHandlers()); > results.append(prepareStyles()); > > - // if no onclick event was provided, put in the cancel script > - if(results.toString().indexOf("onclick=")==-1){ > - results.append(" onclick=\"bCancel=true;\""); > - } > - > - results.append(getElementClose()); > + if (renderJavascript()) { > + String insertStr = " > document.getElementById('"+property+"').value='Cancel'; "; > + insertStr += " > document.getElementById('"+property+"').name='"+property+"'; "; > + insertStr += " this.form.submit(); "; > + > + // if no onclick event was provided, put in the cancel script > + if(results.toString().indexOf("onclick=")==-1){ > + results.append(" onclick=\"bCancel=true; "); > + results.append(insertStr); > + } else { > + int onclickPos = results.toString().indexOf("onclick=\""); > + int nextQuotePos= results.toString().indexOf("\"",onclickPos+10); > + results.insert(nextQuotePos,insertStr); > + } > + > + results.append(getElementClose()); > + > + results.append("<input type=\"hidden\" id=\""+property+"\" value=\"\">"); > + } else { > + // if no onclick event was provided, put in the cancel script > + > + if(results.toString().indexOf("onclick=")==-1){ > + results.append(" onclick=\"bCancel=true;\" "); > + } > + results.append(getElementClose()); > + > + } > > // Render this element to our writer > TagUtils.getInstance().write(pageContext, results.toString()); > @@ -233,12 +293,14 @@ > */ > public void release() { > > - super.release(); > - property = Constants.CANCEL_PROPERTY; > + super.release(); > + property = Constants.CANCEL_PROPERTY; > text = null; > - value = null; > + value = null; > + javascript=null; > > } > > > } > + > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > > --------------------------------------------------------------------- To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
