views:

213

answers:

6

Hello, I am relatively new to Javascript so I'm hoping this is a simple mistake. I building a generic form validation function that is called on the form's onSubmit. The function loops through all the form's child elements, looks for certain classes, and analyzes the contents of the appropriate fields. If it finds something missing or erroneous, it displays the appropriate error message div and returns false, thus preventing the form from being submitted to the php page.

It works well in firefox 3.6.3, but in every other browser I've tested (Safari 4.0.4, Chrome 4.1, IE8) it seems to ignore the onSubmit and jump straight to the php processing page.

HTML CODE:

    <form name='myForm' id='myForm' action='process_form.php' method='post' onSubmit="return validateRequired('myForm')">

   <fieldset class="required radioset">
    <label for='selection1'>
     <input type='radio' name='selection' id='selection1' value='1'/>
     Option 1
    </label>
    <label for='selection2'>
     <input type='radio' name='selection' id='selection2' value='2'/>
     Option 2
    </label>
    <label for='selection3'>
     <input type='radio' name='selection' id='selection3' value='3'/>
     Option 3
    </label>
    <label for='selection4'>
     <input type='radio' name='selection' id='selection4' value='4'/>
     Option 4
    </label>
    <div class='errorBox' style='visibility:hidden'>
     Please make a selection
    </div>
   </fieldset>

   <fieldset class="required checkset">
    <label>
     Choice 1
     <input type='checkbox' name='choices' id='choice1' value='1'/>
    </label>
    <label>
     Choice 2
     <input type='checkbox' name='choices' id='choice2' value='2'/>
    </label>
    <label>
     Choice 3
     <input type='checkbox' name='choices' id='choice3' value='3'/>
    </label>
    <label>
     Choice 4
     <input type='checkbox' name='choices' id='choice4' value='4'/>
    </label>
    <div class='errorBox' style='visibility:hidden'>
     Please choose at least one
    </div>
   </fieldset>


   <fieldset class="required textfield" >
    <label for='textinput1'>
     Required Text:
     <input type='text' name='textinput1' id='textinput1' size='40'/>
    </label>
    <div class='errorBox' style='visibility:hidden'>
     Please enter some text
    </div>
   </fieldset>

   <fieldset class="required email textfield">
    <label for='email'>
     Required Email:
     <input type='text' name='email' id='email' size='40'/>
    </label>
    <div class='errorBox' style='visibility:hidden'>
     The email address you have entered is invalid
    </div>
   </fieldset>


   <div>
    <input type='submit' value='submit'>
    <input type='reset' value='reset'>
   </div>

  </form>

JAVASCRIPT CODE:

    function validateRequired(id){

 var form = document.getElementById(id);
 var errors = 0;
 var returnVal = true;
 for(i = 0; i < form.elements.length; i++){
  var elem = form.elements[i];
  if(hasClass(elem,"required")){

   /*RADIO BUTTON or CHECK BOX SET*/
   if(hasClass(elem,"radioset") || hasClass(elem,"checkset")){
    var inputs = elem.getElementsByTagName("input");
    var check = false;
    for(j = 0; j < inputs.length; j++){
     if(inputs[j].checked){
      check = true;
     }
    }
    if(check == false){
     errors += 1;
     showError(elem);
    } else {
     hideError(elem);
    }
   }

   /*TEXT FIELD*/
   else if(hasClass(elem,"textfield")){
    var input = elem.getElementsByTagName("input");
    if(input[0].value == ""){
     errors += 1;
     showError(elem);
    } else {
     hideError(elem);

     /*EMAIL ADDRESS*/
     if(hasClass(elem,"email")){
      if(isValidEmail(input[0].value) == false){
       errors += 1;
       showError(elem);
      } else {
       hideError(elem);
      }
     }
    }
   }
  }
 }
 if(errors > 0){
  returnVal = false;
 } else {
  returnVal = true;
 }
 return returnVal;}

I know this is a lot of code to look at, but any help would be appreciated. Since it works fine in one browser, Im not sure how to start debugging. Thanks Andrew

A: 

If you have any kind of JavaScript error, the function will not return false, and therefore the default behavior of submitting the data will be triggered.

Try running your code through [http://www.javascriptlint.com/online_lint.php][1] first, then a debugger.

Matthew Smith
replaced a few == with ===, those were the only errors. Still same problem
Logic Artist
A: 

try not to use if(errors > 0)...just in every condition (for wrong input) put return false; and at the end before the last bracket put return true; and better use:

onSubmit="return validateRequired(this)"

and there is no need in this lines (in case you remove the errors var)

 var form = document.getElementById(id);
 var errors = 0;
 var returnVal = true;
cthulhu
Thanks,I'm not sure that will work, since I have to loop through all the form's elements before returning, otherwise it will only display the first error it gets. This was why I put my return at the end. Make sense?
Logic Artist
You could keep `errors` but `returnVal` is quite redundant, just do `return errors===0`. Either way, changing the `id` to simply using `this` directly is a good idea. Even better is to write the handler directly from JavaScript (`document.getElementById('myForm').onsubmit= function() { ...validation stuff... }`), and use `this` and `return` directly inside that, instead of using nasty inline event handler attributes.
bobince
I have changed it to onSubmit="return validateRequired(this)". No difference, still works in firefox, but nothing else
Logic Artist
I have changed it to...<script type="text/javascript" defer="defer"> document.getElementById('myForm').onsubmit = function(){ return validateRequired('myForm'); }</script><form name='myForm' id='myForm' action='process_form.php' method='post' >...And now it doesn't work at all. All browsers just jump to process_form.php
Logic Artist
You must naturally put the `<script>` below the `<form>` it references, otherwise the object won't exist yet. By `...validation stuff...` I mean the current contents of `validateRequired`, changed as cthulu said to use `this` instead of trying to get a form element by `id`. There is no need for the extra wrapper function that does nothing but call `validateRequired`.
bobince
+1  A: 
for(i = 0; i < form.elements.length; i++){
    var elem = form.elements[i];
    if(hasClass(elem,"required")){

The problem is that your required and other classes are put on the <fieldset> element.

Fieldset elements are included in the form.elements collection on IE, Firefox and Opera, but not WebKit browsers (Chrome, Safari). It is these browsers where your form fails for me.

It has always been a weird quirk that <fieldset> was included in the elements collection. The DOM Level 2 HTML spec states that only ‘form control elements’ should be present in the collection, and by HTML4's definition that would seem not to include fieldsets, which have no control name or value.

You could perhaps change your code to use getElementsByTagName to pick up the fieldsets instead:

var fieldsets= form.getElementsByTagName('fieldset');
for (var i= 0; i<fieldsets.length; i++) {
    var elem= fieldsets[i];
bobince
Thank you. Im not sure how you could have tested my form, though, since I did not include all my functions here. hasClass(), isValidEmail(), showError(), hideError()... these are functions I created
Logic Artist
oh, it's pretty obvious what they were supposed to do, and mock up a stub.
bobince
This worked, you hit the nail on the head. Thanks! I altered it so it looks for "fieldset" elements only and now it works in all 4 browsers. My original thought was I could add the "required" class to any form element, but I suppose requiring it be a fieldset is workable.
Logic Artist
+1  A: 

I would not use hasClass. Here's another way to try that might work better for you:

var node_list = document.getElementsByTagName('input');
for (var i = 0; i < node_list.length; i++) {
    var node = node_list[i];

    if (node.getAttribute('type') == 'text') {
        // do something here with a <input type="text" .../>
        alert(node.value);
    }
} 

I know that IE has problems getting the classes from some elements which have multiple classes associated with them. Regardless, this is a handy function.

fakeit
A: 

Not the cause of your problem, I'm sure, but it's best not to serve a set of radio buttons without one of them selected.

In your particular case, if you know that you set one when you serve the page, you don't need a "required" check; there's no way the user can get the radio buttons into a state where none are selected. Pick a sensible default, make it the first option, and make it selected. Simplify your life :)

From the W3C:

http://www.w3.org/TR/html401/interact/forms.html#radio

If no radio button in a set sharing the same control name is initially "on", user agent behavior for choosing which control is initially "on" is undefined. Note. Since existing implementations handle this case differently, the current specification differs from RFC 1866 ([RFC1866] section 8.1.2.4), which states:

At all times, exactly one of the radio 
buttons in a set is checked. If
none of the <INPUT> elements of a set
of radio buttons specifies `CHECKED',
then the user agent must check the
first radio button of the set
initially.

Since user agent behavior differs, authors should ensure that in each set of radio buttons that one is initially "on".

Ed Daniel
Thats a good point, thank you.
Logic Artist
A: 

Back to basics for a second: You say it "seems to ignore the onsubmit". That leaves three possibilities that I can think of:

  1. Your function is never called
  2. It's called, and is bombing out part-way through due to an error
  3. It's called, and isn't doing what you think it is, always returning true

It's not clear from your question which it is, so if I were you I'd start debugging this by putting an alert at the beginning of the function to see whether IE's calling it at all - then move that alert down and run the validation again, and see where it stops appearing (any error must be above that point).

I'd also want to alert the return value from the place it was called, to see what was coming back. If it's always true, then your code is working but not doing what you think it does.

From there, you at least have a clearer grasp of what's going on, if not an exact block of code to be scrutinising.

Ed Daniel