Form validation problem

[unplugged_web]

I have a form that I'm trying to use JavaScript to validate. I've got:

<script type="text/javascript">function validate_email(field,alerttxt){with (field){apos=value.indexOf("@");dotpos=value.lastIndexOf(".");if (apos<1||dotpos-apos<2)   {alert(alerttxt);return false;}else {return true;}{if (value==null||value=="")  {alert(alerttxt);return false;}else {return true}}}function validate_form(thisform){with (thisform){if (validate_email(email,"Please provide a valid email address.")==false)  {email.focus();return false;}{if (validate_required(comments,"Email must be filled out!")==false)  {comments.focus();return false;}}}</script>

but that doesn't work. I used:

<script type="text/javascript">function validate_email(field,alerttxt){with (field){apos=value.indexOf("@");dotpos=value.lastIndexOf(".");if (apos<1||dotpos-apos<2)   {alert(alerttxt);return false;}else {return true;}}}function validate_form(thisform){with (thisform){if (validate_email(email,"Please provide a valid email address.")==false)  {email.focus();return false;}}}</script>

and that did work, but I ideally want to make sure that both the email and comment boxes to be filled in.The code for the form is:

<form id="form1" name="form1" method="post" action="results.php" onSubmit="return validate_form(this)"><input type="text" name="name" /><input type="text" name="phone" /><input type="text" name="email" /><input name="month" type="text" size="10" /><input name="year" type="text" size="4" /><textarea name="comments" cols="30" rows="4"></textarea><input name="submit" type="image" src="image/submit.png" height="16" width="64" /></form>

Thanks

[jeffman]

Let's just clean this up. I've added IDs to the important form elements and used them to grab the references we need. For the email validation I've used a regular expression that I'll be happy to explain if anyone wants.

<script type="text/javascript">  function validate_form()	{	 var e = document.getElementById('email');	  var c = document.getElementById('comments');	 var eAlert = "Please provide a valid email address.";	 var cAlert = "Please add comments.";	   	 if (e.value.match(/[@.]/g).length < 2) {			  alert (eAlert);			  return false;	  }	  if (!e.value) {			  alert (eAlert);			  return false;	  }	  if (!c.value) {			alert (cAlert);			return false;	  }	 return true; // I think we need this? Don't remember.	}</script>

If I had more time, I'd hack a way of concatenating all your alert strings into one message so you only had to alert your client once. I'm sure you can do that on your own, though, and I recommend it.Here's the modified form. Just notice the IDs:

<form id="form1" name="form1" method="post" action="results.php" onSubmit="return validate_form()">	<input type="text" name="name" />	<input type="text" name="phone" />	<input type="text" name="email" id="email"/>	<input name="month" type="text" size="10" />	<input name="year" type="text" size="4" />	<textarea name="comments" id="comments" cols="30" rows="4"></textarea>	<input name="submit" type="image" src="image/submit.png" height="16" width="64" /></form>

[unplugged_web]

Thanks. I tried that, but it just sent the form and didn't display any error message at all.

[jeffman]

Here is some revised code.

   function validate_form(){		var e = document.getElementById('email');		var c = document.getElementById('comments');		var eAlert = "Please provide a valid email address.";		var cAlert = "Please add comments.";		var em;		 if (!e.value) {				 alert (eAlert);				 return false;		 }		 em = e.value.match(/[@.]/g);		 if ((!em) || (em.length < 2)) {				 alert (eAlert);				 return false;		 }		 if (!c.value) {			   alert (cAlert);			   return false;		 }		 return true;   }

Well, I hacked the original out too quickly and didn't test it. The first mistake was testing for e.value AFTER I tried matching e.value. If there's no value to match, we get an error. So reversing the first two conditions was an important step. Likewise, testing for the length of the match if the match is undefined was a little dumb. That also throws an error. So here, we test for the match and then test for its length. (I suppose the first two conditionals could be combined, but this way everything is pretty readable.)I DID check this code and it works. I hope it works for you.

[justsomeguy]

You might also want to tune the email address expression. It needs to pick up all of these:user@domain.comfname.lname@domain.comfname.lname@sub2.sub1.domain.tld2.tld1

[unplugged_web]

Brilliant, thanks that work great