optimize my first javascript validation code

[helenp]

Hi,
Thanks by everything you have showned me I have done my first validation script.
It uses ids, I know there are smarter ways but I like it as it is mine and I keep learning.

I am validating the inputs are empty and using a focus to jump up to the input as the form is very long.
I wonder what should I use to delete the innerhtml message and focus when a value is inerted into the box?
Suppose maybe onchange then innerthml = ''?

This is my script so far, I wanted to only write the css class ones, but as the id does not exist yet I did not manage.
This is what I have,
Any opinion, something I should improve?:

<script type="text/javascript"> 
function OnSubmitBooking()
{

if(document.Booking.name.value == '')
 {
divresult = document.getElementById('namebox')
divresult.className = 'validation'
divresult.innerHTML = 'Please fill in name';
document.Booking.name.focus();
return false;
 }
 if(document.Booking.passport.value == '')
 {
divresult = document.getElementById('passportbox')
divresult.className = 'validation'
divresult.innerHTML = 'Please fill in your identification';
document.Booking.passport.focus();
return false;
 }
if(document.Booking.emailtrue.value == '')
 {
divresult = document.getElementById('emailtruebox');
divresult.className = 'validation'
divresult.innerHTML = 'Please fill in email';
document.Booking.emailtrue.focus();
return false;
 }
 if(document.Booking.repeat_email.value == '')
 {
divresult = document.getElementById('repeat_emailbox');
divresult.className = 'validation' 
divresult.innerHTML = 'Please repeat email';
document.Booking.repeat_email.focus();
return false;
 }
if(document.Booking.telmobile.value == '')
 {
divresult = document.getElementById('telmobilebox');
divresult.className = 'validation'
divresult.innerHTML = 'Please fill in mobile to bring on holiday';
document.Booking.telmobile.focus();
return false;
 }
if(document.Booking.emailtrue.value != document.Booking.repeat_email.value)
 {
divresult = document.getElementById('repeat_emailbox');
divresult.className = 'validation'
divresult.innerHTML = 'Email and repeat email are not equal';
return false;
 }
 if(document.Booking.test.value =='')
 {
divresult = document.getElementById('resultbooking');
divresult.className = 'validation'
divresult.innerHTML = 'Please answer the security question';
document.Booking.test.focus();
return false;
 }
 if (isNaN(document.Booking.test.value)) 
 {
divresult.className = 'validation'
divresult.innerHTML = 'The answer is invalid, use only numbers in security question';
document.Booking.test.focus();
return false;
 }
 
 else{
 divresult.style.color="blue";
 divresult.innerHTML = 'Please Wait...';} 
}
  </script>

[helenp]

Tried to add the onchange function inside the other function in the first input but does not work:

function OnSubmitBooking()
{

if(document.Booking.name.value == '')
 {
divresult = document.getElementById('namebox')
divresult.className = 'validation'
divresult.innerHTML = 'Please fill in name';
document.Booking.name.focus();
return false;
function quitarfocus()
{
divresult.innerHTML.style.display = "none"}
 }
html:
<input id="name" name="name" title="Please enter your name" onchange="quitarfocus()" type="text" size="50">
<div id="namebox"></div> 

[Fotiman]

Some suggestions.

1. Look for repetition. If you're using document.Booking over and over, consider putting it into a shorter variable:

var f = document.Booking; // the form

2. Always declare variables with "var", otherwise you're creating globals, which can have unintended consequences.

var divresult;

3. document.Booking.name.value
Having a form element named "name" can be problematic, because the form element itself can have a name attribute. If you can, consider giving the "name" element a different name. If you can't, then do this instead:

document.Booking.elements['name'].value

4. Each of those "if" bodies looks similar. You could probably make a function that takes the name and message, and reuse it for all of those to prevent duplication.

[helenp]

Thanks for that,
think most are covered now.
I have added an ajax at the end, and it works but always return false.
I need that if there is no responsetext to validate true:

function objetoAjax(){
var xmlhttp=false;
try {
xmlhttp = new ActiveXObject("Msxml2.XMLHTTP");
} catch (e) {
 
try {
xmlhttp = new ActiveXObject("Microsoft.XMLHTTP");
} catch (E) {
xmlhttp = false;
}
}
 
if (!xmlhttp && typeof XMLHttpRequest!='undefined') {
 xmlhttp = new XMLHttpRequest();
}
return xmlhttp;
}

//Función para recoger los datos del formulario y enviarlos por post 
function OnSubmitBooking()
{
if(document.Booking.emailtrue.value != document.Booking.repeat_email.value)
 {
divresult = document.getElementById('repeat_emailbox');
divresult.className = 'validation'
divresult.innerHTML = 'Email and repeat email are not equal';
 document.Booking.repeat_email.focus();
return false;
 }
 var info = {
   names  : ['namebox'  , 'Please fill in name' ],
   passport : ['passportbox' , 'Please fill in your identification' ],
   emailtrue : ['emailtruebox' , 'Please fill in email' ],
   repeat_email: ['repeat_emailbox', 'Please repeat email' ],
   telmobile : ['telmobilebox' , 'Please fill in mobile to bring on holiday' ],
   test  : ['resultbooking' , 'Please answer the security question' ]
  }
  for( key in info ) {
   if(document.Booking[key].value == '')
   {
    divresult = document.getElementById(info[key][0])
    divresult.className = 'validation'
    divresult.innerHTML = info[key][1];
    document.Booking[key].focus();
    return false;
   }
  }
divresult = document.getElementById('resultbooking');
 //recogemos los valores de los inputs

enviar = document.solicitud.enviar.value;
 question = document.solicitud.question.value;
 answer = document.solicitud.answer.value;
 test = document.solicitud.test.value;
 var ajaxcaptcha = objetoAjax();
 ajaxcaptcha.open("POST", "captchaajax.php", true);
 ajaxcaptcha.onreadystatechange = function () {
 if (ajaxcaptcha.readyState == 4) {
  divresult.innerHTML = ajaxcaptcha.responseText
  <!-- LimpiarCampos();-->
 }
}
 ajaxcaptcha.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
 ajaxcaptcha.send("test=" + test + "&enviar=" + enviar + "&answer=" + answer + 
  "&question=" + question + "");
 return false;
}

This is the php page:

if (isset($_POST['enviar'])){
$question = $_POST['question']; 
$answer = $_POST['answer']; 
$test = $_POST['test']; 
 $enviar = $_POST['enviar'];
$result2 = $dbh->query("SELECT question, answer FROM captcha ORDER BY RAND() LIMIT 1");
if($row = $result2->fetch()) {
$question = $row["question"];
$answer = $row["answer"];
if ($test != $answer) {
echo "The answer of the security question is not correct";
}
} 
}

[helenp]

Puf,
try to return true if valid,
any idea how to do this?
I want to return true (submit the form if it validates)

enviar = document.solicitud.enviar.value;
 question = document.solicitud.question.value;
 answer = document.solicitud.answer.value;
 test = document.solicitud.test.value;
 var ajaxcaptcha = objetoAjax();
 ajaxcaptcha.open("POST", "/pasarela/captchaajax.php", true);
 ajaxcaptcha.onreadystatechange = function () {
 if (ajaxcaptcha.readyState == 4) {
  if (ajaxcaptcha.responseText === "ok") {
return true;
  } else {
  divresult.innerHTML = "The answer to the security question was not correct"
  <!-- LimpiarCampos();-->
  }
 }
}

 ajaxcaptcha.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
 ajaxcaptcha.send("test=" + test + "&enviar=" + enviar + "&answer=" + answer + 
  "&question=" + question + "");
 return false;
}

I have simplified the php page as no need to do a query again and it works when I dont try to get a true valid, but I cant manage the ajax part:

if (isset($_POST['enviar'])){
$answer = $_POST['answer']; 
$test = $_POST['test']; 
  $enviar = $_POST['enviar'];


if ($test = $answer) {
echo "ok";
}
}

[Fotiman]

Tried to add the onchange function inside the other function in the first input but does not work:

You wouldn't want that defined within the other function, you'd want it to be a sibling function. Like this:

function OnSubmitBooking()
{
 ...
}
function quitarfocus()
{
 ...
}

Also, ideally you should not use inline event handlers. Instead, it's better to assign the event handlers from the JavaScript. So instead of this:
<input id="name" name="name" title="Please enter your name" onchange="quitarfocus()" type="text" size="50">

You could do something like this within your script:
var n = document.getElementById('name');
n.onchange = quitarfocus;

Even better, though, you might attach a single onchange event handler to a parent element of all your inputs, so you only need 1 handler total instead of 1 for each element. Your handler would then need to inspect the event to see which input field triggered it.

[Fotiman]

AJAX requests are typically "Asynchronous". As such, you can't expect a synchronous method to be able to return the value from an AJAX request. Now, you *could* make the request synchronous by doing something like this:

ajaxcaptcha.open("POST", "/pasarela/captchaajax.php", false);
if (ajaxcaptcha.readyState == 4) {
  if (ajaxcaptcha.responseText === "ok") {
    return true;
  } else {
    divresult.innerHTML = "The answer to the security question was not correct"
    <!-- LimpiarCampos();-->
  }
}

In general, you should rarely if ever use synchronous requests.

[Fotiman]

Alternatively, you could always return false from your onsubmit method, then in the AJAX request, call form.submit() if the request is successful.

[helenp]

Alternatively, you could always return false from your onsubmit method, then in the AJAX request, call form.submit() if the request is successful.

This is been trying, been reeeding and if I understood correctly instead of true I declare false here:
ajaxcaptcha.open("POST", "captchaajax.php", false);
Is this correct?

Will try the onchange when I finished the Ajax return true

[Fotiman]

This is been trying, been reeeding and if I understood correctly instead of true I declare false here:
ajaxcaptcha.open("POST", "captchaajax.php", false);

That makes it synchronous (my first example). But instead you could do something like this:

function OnSubmitBooking() {
  ...
  ajaxcaptcha.open("POST", "captchaajax.php", false);
  ajaxcaptcha.onreadystatechange = function () {
    if (ajaxcaptcha.readyState == 4) {
      if (ajaxcaptcha.responseText === "ok") {
        document.Booking.submit();
      } else {
        divresult.innerHTML = "The answer to the security question was not correct"
        <!-- LimpiarCampos();-->
      }
    }
  };
  return false; // always return false here to prevent default form submit
}

Though you may want to populate a hidden form with the values from the form the user sees and submit that instead. That way, if there is any time between your AJAX call and the response, any changes the user makes to the form could be ignored.

[helenp]

document.Booking.submit();

This is ever seen, thanks hope that will do it.
Will let you know, late dinnertime ;)

[helenp]

That makes it synchronous (my first example). But instead you could do something like this:

We answered at same time lol :)

[helenp]

In general, you should rarely if ever use synchronous requests.

Then I should pass a big amount of vars by the Ajax doing it with php the normal way, redirection.
Was trying to avoid passing all the vars.
Better to be on the safe side

[Fotiman]

Ooops, I meant this:

function OnSubmitBooking() {
  ...
  ajaxcaptcha.open("POST", "captchaajax.php", true); // async
  ajaxcaptcha.onreadystatechange = function () {
    if (ajaxcaptcha.readyState == 4) {
      if (ajaxcaptcha.responseText === "ok") {
        document.Booking.submit();
      } else {
        divresult.innerHTML = "The answer to the security question was not correct"
        <!-- LimpiarCampos();-->
      }
    }
  };
  return false; // always return false here to prevent default form submit
}

[helenp]

Thanks, been struggling all day not able to understand why end did not work,.....wrong form name puf.
Anyway it works now, however not sure I can do it using this: document.Booking.submit();
That way it submits through Ajax and the form submits to itself and if the var in the submit button exists a php redirection is done.

Is it possible to send a var to php in this or some other way?
document.Booking.submit();

Looks like I will have to pass all vars by ajax, its a very long form so I tried to be smarter and only send the vars I needed to validate and let the rest for php to do.

[Fotiman]

I'm not sure I understand what you're asking or what you're trying to achieve.

[helenp]

Same as this:
ajaxcaptcha.send("test=" + test + "&enviar=" + enviar + "&answer=" + answer +
"&question=" + question + "");
however the contrary, send back to the page where the form is.
The form is executed using: document.Booking.submit();
however the form point to the page where the form is,
in order to do the php header redirection with its sessions I need to get at least the var enviar back.
I suppose as the form is not normally submitted as I do this: document.Booking.submit();
the var in the form input:
<input class="boton roundedcorner border" type="submit" name="enviar" value="Confirm booking and pay deposit now">
is being lost.
So the php on the page where the form is: if (isset($_POST['enviar'])){
is not executed.

[Fotiman]

What if you just insert a hidden input with the name "enviar" into the form in your JavaScript? So before you call document.Booking.submit(), do something like this:

var enviar = document.createElement('input');
enviar.type = 'hidden';
enviar.name = 'enviar';
enviar.value = true;
document.Booking.appendChild(enviar);
document.Booking.submit();

[helenp]

What if you just insert a hidden input with the name "enviar" into the form in your JavaScript? So before you call document.Booking.submit(), do something like this:

That works perfect, thanks, maybe I could have found that in my friend google.

[helenp]

Also, ideally you should not use inline event handlers. Instead, it's better to assign the event handlers from the JavaScript. So instead of this:
<input id="name" name="name" title="Please enter your name" onchange="quitarfocus()" type="text" size="50">

You could do something like this within your script:
var n = document.getElementById('name');
n.onchange = quitarfocus;

Even better, though, you might attach a single onchange event handler to a parent element of all your inputs, so you only need 1 handler total instead of 1 for each element. Your handler would then need to inspect the event to see which input field triggered it.

Thanks for all, on to next step, cleaning up the error messages, manage to get it work my way (at least the first).

This does not work:

var n = document.getElementById('repeat_email');
n.onchange = quitarfocus();
 function quitarfocus()
{

document.getElementById("repeat_emailbox").style.display = "none";
 
}
 
function OnSubmitBooking()
{
if(document.Booking.emailtrue.value != document.Booking.repeat_email.value)
 {

This does not work either:

 function quitarfocus()
{
var n = document.getElementById('repeat_email');
n.onchange = quitarfocus();
document.getElementById("repeat_emailbox").style.display = "none";
 
}

function OnSubmitBooking()
{
if(document.Booking.emailtrue.value != document.Booking.repeat_email.value)
 {
 document.getElementById('repeat_emailbox').style.display = "block";

However this works (at least in firefox):

<input id="repeat_email" onchange="quitarfocus()" name="repeat_email" title="Please repeat your email" size="61" type="text">
 <div id="repeat_emailbox"></div> 

 function quitarfocus()
{

var n = document.getElementById('repeat_email');
 if (n != '')
{
document.getElementById("repeat_emailbox").style.display = "none";
} 
}

function OnSubmitBooking()
{
if(document.Booking.emailtrue.value != document.Booking.repeat_email.value)
{
document.getElementById('repeat_emailbox').style.display = "block"; 

This is what I have so fare:

function objetoAjax(){
var xmlhttp=false;
try {
xmlhttp = new ActiveXObject("Msxml2.XMLHTTP");
} catch (e) {
 
try {
xmlhttp = new ActiveXObject("Microsoft.XMLHTTP");
} catch (E) {
xmlhttp = false;
}
}
 
if (!xmlhttp && typeof XMLHttpRequest!='undefined') {
 xmlhttp = new XMLHttpRequest();
}
return xmlhttp;
}
 function quitarfocus()
{

var n = document.getElementById('repeat_email');
 if (n != '')
{
document.getElementById("repeat_emailbox").style.display = "none";
} 
}
 
function OnSubmitBooking()
{
if(document.Booking.emailtrue.value != document.Booking.repeat_email.value)
 {
 document.getElementById('repeat_emailbox').style.display = "block";
divresult = document.getElementById('repeat_emailbox');
divresult.className = 'validation'
divresult.innerHTML = 'Email and repeat email are not equal';
 document.Booking.repeat_email.focus();
return false;
 }
 var info = {
 names : ['namebox' , 'Please fill in name' ],
 passport : ['passportbox' , 'Please fill in your identification' ],
 emailtrue : ['emailtruebox' , 'Please fill in email' ],
 repeat_email: ['repeat_emailbox', 'Please repeat email' ],
 telmobile : ['telmobilebox' , 'Please fill in mobile to bring on holiday' ],
 test : ['resultbooking' , 'Please answer the security question' ]
 }
 for( key in info ) {
 if(document.Booking[key].value == '')
 {
 divresult = document.getElementById(info[key][0])
 divresult.className = 'validation'
 divresult.innerHTML = info[key][1];
 document.Booking[key].focus();
 return false;
 }
 }
divresult = document.getElementById('resultbooking');

var enviar = document.Booking.enviar.value,
question = document.Booking.question.value,
answer = document.Booking.answer.value,
test = document.Booking.test.value;
 var ajaxcaptcha = objetoAjax();
 ajaxcaptcha.open("POST", "captchaajax.php", true);
 ajaxcaptcha.onreadystatechange = function () {
 if (ajaxcaptcha.readyState==1 || ajaxcaptcha.readyState==2 || ajaxcaptcha.readyState==3)
 {
divresult.className = 'validationok'
 divresult.innerHTML = 'Please wait...';
 }
 if (ajaxcaptcha.readyState == 4) {
 if (ajaxcaptcha.responseText === "Loading, please wait") {
 var enviar = document.createElement('input');
enviar.type = 'hidden';
enviar.name = 'enviar';
enviar.value = true;
document.Booking.appendChild(enviar);
 document.Booking.submit();
 } else {
 divresult.className = 'validation'
 divresult.innerHTML = ajaxcaptcha.responseText
 }
 }
 }

 ajaxcaptcha.setRequestHeader("Content-Type", "application/x-www-form-urlencoded");
 ajaxcaptcha.send("test=" + test + "&enviar=" + enviar + "&answer=" + answer + 
 "&question=" + question + "");
 return false;
}

[Fotiman]

n.onchange = quitarfocus();

You don't want to include the parenthesis at the end. Otherwise you're executing quitarfocus() immediately and assigning the return value to n.onchange instead of assigning the function itself to onchange. So try this instead:
n.onchange = quitarfocus;

[helenp]

n.onchange = quitarfocus;

Yes I managed to get it to work with one of the vars.
However I just decided to leave it, and just change the css so it looks ok.
Thanks for everything I learnt a lot.