Problem with pretty simple validation

Re: Problem with pretty simple validation

Marc Llenas wrote:[color=blue]
> Hi there,
>
> I'm stuck on a validation function for a form and I cannot figure out what
> the problem is. The page is in ASP. Any ideas?
>
> if (.... (form.CboType.v alue==7))[/color]

This may not be only problem, but the universal way to read a select
element is by indexing its options array with its selectedIndex
property:

form.CboType.op tions[ form.CboType.se lectedIndex ].value

--
S.C.

Re: Problem with pretty simple validation

Marc Llenas wrote:[color=blue]
> Hi there,
>
> I'm stuck on a validation function for a form and I cannot figure out what
> the problem is. The page is in ASP. Any ideas?
>
> The function being called is:[/color]

When posting code, manually wrap it at about 70 characters to prevent
wrapping, which otherwise introduces more errors that must be fixed
before help can be provided.
[color=blue]
>
> <script language="JavaS cript" type="text/javascript">[/color]

The language attribute is depreciated, type is required:

<script type="text/javascript">

[color=blue]
> function checkform ( form )
> {
> if (form.txtDate.v alue == "")
> {
> alert( "Si us plau, seleccioneu la data del festiu sol·licitat per
> l'usuari" );[/color]

Here is some error-inducing wrapping, more occurs elsewhere:

alert( "Si us plau, seleccioneu la data del festiu"
+ " sol·licitat per l'usuari" );
[color=blue]
> form.txtDate.fo cus();
> return false;
> }
> if (form.CboType.v alue == "")
> {
> alert( "Si us plau, seleccioneu el tipus de festiu sol·licitat per
> l'usuari" );
> form.CboType.fo cus();
> return false;
> }
>
> if(!isDate(form .txtDate.value) )[/color]

Leaving in this line without the code for isDate() causes an error that
must be fixed in order to make your code run. Either remove it or
include a dummy isDate().
[color=blue]
> {
> alert("Format de data invàlida (dd-mm-aaaa)");
> form.txtDate.fo cus();
> return false;
> }
> if ((form.txtDetai ls.value=="") && (form.CboType.v alue==7))[/color]

For some browsers, the value of a select is the value of the selected
option (e.g. Firefox), but for other browsers (e.g. IE) you have to use
the selectedIndex value:

if (form.txtDetail s.value==""
&& form.CboType[form.CboType.se lectedIndex].value==7) {
[color=blue]
> {
> alert( "Si us plau, introdui el motiu de la sol·licitud" );
> form.txtDetails .focus();
> return false;
> }
> if ((form.txtDetai ls.value=="") && (form.CboType.v alue==8))
> {
> alert( "Si us plau, introdui el motiu de la sol·licitud" );
> form.txtDetails .focus();
> return false;[/color]

As you noted, these tests could be combined:

var t = form.CboType[form.CboType.se lectedIndex].value;
if ((8==t || 7==t) && "" == form.txtDetails .value){
...
[color=blue]
> }
> return true;
> }
> </SCRIPT>
>
> The function is called by:
>
> <form method="post" name="frmMain" action="process .asp" onsubmit="retur n
> checkform(this) ;" >[/color]

Here wrapping has caused an issue with testing your code. The script
parser sees - return - then a carriage return, followed by a statement
so a semi-colon is inserted. The return executes without doing the
checkform() bit and without an error - debugging your code is made that
much more difficult.

<form method="post" name="frmMain" action="process .asp"
onsubmit="retur n checkform(this) ;" >

[...]
[color=blue]
> <INPUT TYPE="text" NAME="txtDate" class="Days" STYLE="width: 140px"
> readonly>
> <A HREF="#" onClick="if(oDP )oDP.open(frmMa in.txtDate);ret urn false;">[/color]

What does oDP do? Presumably it creates a pop-up with a date selector -
for the sake of testing, why not hard-code a valid value in the onclick,
or in the readonly txtDate form control?

Your reference to the form uses 'frmMain' as a global variable, that
will work only in IE:

<A HREF="#" onClick="
if(oDP)oDP.open (document.forms['frmMain'].elements['txtDate']);
return false;
">

Many would say that you should use the image element with a pointer
cursor and remove the A element. Or better, use a button and then the
reference to the form becomes shorter:

<input type="button" value="Calendar " onClick="
if(oDP)oDP.open (this.form.elem ents['txtDate']);
return false;
">

[...]


--
Rob

Re: Problem with pretty simple validation

Woa!, thanks a bunch Rob,

This is probably the most in-dept review of a piece of code I've ever seen.
Will modify the code as per your suggestions. Thanks a million.

Marc

"RobG" <rgqld@iinet.ne t.au> escribió en el mensaje
news:vuE2f.892$ uQ6.44184@news. optus.net.au...[color=blue]
> Marc Llenas wrote:[color=green]
>> Hi there,
>>
>> I'm stuck on a validation function for a form and I cannot figure out
>> what the problem is. The page is in ASP. Any ideas?
>>
>> The function being called is:[/color]
>
> When posting code, manually wrap it at about 70 characters to prevent
> wrapping, which otherwise introduces more errors that must be fixed before
> help can be provided.
>[color=green]
>>
>> <script language="JavaS cript" type="text/javascript">[/color]
>
> The language attribute is depreciated, type is required:
>
> <script type="text/javascript">
>
>[color=green]
>> function checkform ( form )
>> {
>> if (form.txtDate.v alue == "")
>> {
>> alert( "Si us plau, seleccioneu la data del festiu sol·licitat per
>> l'usuari" );[/color]
>
> Here is some error-inducing wrapping, more occurs elsewhere:
>
> alert( "Si us plau, seleccioneu la data del festiu"
> + " sol·licitat per l'usuari" );
>[color=green]
>> form.txtDate.fo cus();
>> return false;
>> }
>> if (form.CboType.v alue == "")
>> {
>> alert( "Si us plau, seleccioneu el tipus de festiu sol·licitat per
>> l'usuari" );
>> form.CboType.fo cus();
>> return false;
>> }
>>
>> if(!isDate(form .txtDate.value) )[/color]
>
> Leaving in this line without the code for isDate() causes an error that
> must be fixed in order to make your code run. Either remove it or include
> a dummy isDate().
>[color=green]
>> {
>> alert("Format de data invàlida (dd-mm-aaaa)");
>> form.txtDate.fo cus();
>> return false;
>> }
>> if ((form.txtDetai ls.value=="") && (form.CboType.v alue==7))[/color]
>
> For some browsers, the value of a select is the value of the selected
> option (e.g. Firefox), but for other browsers (e.g. IE) you have to use
> the selectedIndex value:
>
> if (form.txtDetail s.value==""
> && form.CboType[form.CboType.se lectedIndex].value==7) {
>[color=green]
>> {
>> alert( "Si us plau, introdui el motiu de la sol·licitud" );
>> form.txtDetails .focus();
>> return false;
>> }
>> if ((form.txtDetai ls.value=="") && (form.CboType.v alue==8))
>> {
>> alert( "Si us plau, introdui el motiu de la sol·licitud" );
>> form.txtDetails .focus();
>> return false;[/color]
>
> As you noted, these tests could be combined:
>
> var t = form.CboType[form.CboType.se lectedIndex].value;
> if ((8==t || 7==t) && "" == form.txtDetails .value){
> ...
>[color=green]
>> }
>> return true;
>> }
>> </SCRIPT>
>>
>> The function is called by:
>>
>> <form method="post" name="frmMain" action="process .asp" onsubmit="retur n
>> checkform(this) ;" >[/color]
>
> Here wrapping has caused an issue with testing your code. The script
> parser sees - return - then a carriage return, followed by a statement so
> a semi-colon is inserted. The return executes without doing the
> checkform() bit and without an error - debugging your code is made that
> much more difficult.
>
> <form method="post" name="frmMain" action="process .asp"
> onsubmit="retur n checkform(this) ;" >
>
> [...]
>[color=green]
>> <INPUT TYPE="text" NAME="txtDate" class="Days" STYLE="width:
>> 140px" readonly>
>> <A HREF="#" onClick="if(oDP )oDP.open(frmMa in.txtDate);ret urn
>> false;">[/color]
>
> What does oDP do? Presumably it creates a pop-up with a date selector -
> for the sake of testing, why not hard-code a valid value in the onclick,
> or in the readonly txtDate form control?
>
> Your reference to the form uses 'frmMain' as a global variable, that will
> work only in IE:
>
> <A HREF="#" onClick="
> if(oDP)oDP.open (document.forms['frmMain'].elements['txtDate']);
> return false;
> ">
>
> Many would say that you should use the image element with a pointer cursor
> and remove the A element. Or better, use a button and then the reference
> to the form becomes shorter:
>
> <input type="button" value="Calendar " onClick="
> if(oDP)oDP.open (this.form.elem ents['txtDate']);
> return false;
> ">
>
> [...]
>
>
> --
> Rob[/color]