What is wrong with this validation?

Question

Hi.

I have had several threads about validating my textarea; for some reason it won't validate. The function that validates doesn't seem to be called at all.

Here is the form simplified:

<form name="annonsera" id="annonsera" method="post"
        enctype="multipart/form-data" onSubmit="return validateForm();">
<textarea name="annonsera_des" cols="45" rows="12" id="annonsera_des"></textarea>
</form>

And here is the Javascript that gets the form textarea's value:

var description = document.getElementById("annonsera_des");

And here is the javascript that calls the function to validate all fields etc...

function validateForm() {
    var name = document.getElementById("annonsera_name");
    var tel = document.getElementById("annonsera_tel");
    var email = document.getElementById("annonsera_email");
    var area = document.getElementById("annonsera_area");
    var community = document.getElementById("annonsera_area_community");
    var category = document.getElementById("annonsera_category");
    var subcats = document.getElementById("annonsera_subcats").getElementsByTagName("select");
    var headline = document.getElementById("annonsera_headline");
    var description = document.getElementById("annonsera_des");

    if (nameValid(name)){
        if (telValid(tel)){
            if (emailValid(email)){
                if (areaValid(area)){
                    if (communityValid(community)){
                        if (categoryValid(category)){
                            if (subcatsValid(subcats)){
                                if (headlineValid(headline)){
                                    if(descriptionValid(des)){
                                        return true;
                                    }
                                }
                            }
                        }
                    }
                }
            }
        }
    }
    return false;
}

And here is the function descriptionValid():

function descriptionValid(fld){
    if (fld.value=="") {
        document.getElementById("annonsera_descriptionempty_error").style.display='block';
        document.getElementById("annonsera_description").focus();
        document.getElementById("annonsera_descriptionshort_error").style.display='none'; 
        return false;
    }
    else if (fld.value.length<3){
        document.getElementById("annonsera_descriptionshort_error").style.display='block';
        document.getElementById("annonsera_description").focus();
        document.getElementById("annonsera_descriptionempty_error").style.display='none'; 
        return false;
    }
    document.getElementById("annonsera_descriptionempty_error").style.display='none'; 
    document.getElementById("annonsera_descriptionshort_error").style.display='none'; 
    return true;
}

So, what is wrong with this code?

NOTE: I have added alert boxes inside the descriptionValid() function, and none of them get displayed, so basically the function doesn't get called for some reason. The form just submits. The strange part is that I am validating all other fields and drop lists using exactly the same method and it works fine.

Answer 1

You have way too many nested if's.

Answer 2

I wont nag about your coding-style, as I'm sure there will be lots of that anyway :)

Use Firebug to set a breakpoint and then step through the execution of your script to see what is wrong.

Answer 3

In the original function, you say:

var description = document.getElementById("annonsera_des");

But later, in the validation function, you use:

document.getElementById("annonsera_description")

Which one is it? des or description?

Update, with another idea: I'm not sure if you can do onClick="return validateForm();" - what happens if you do onClick="javascript:validateForm(); or just onClick="validateForm();"?

Answer 4

I don't know what is wrong with the code offhand, but you really, really should not have a function whose purpose is to check if a field is valid have side effects. Instead of having this:

function descriptionValid(fld){
    if (fld.value=="") {
        document.getElementById("annonsera_descriptionempty_error").style.display='block';
        document.getElementById("annonsera_description").focus();
        document.getElementById("annonsera_descriptionshort_error").style.display='none';       
        return false;
    }

you should just have this:

function isDescriptionValid(fld) {
    return (!fld.value=="");
}

and then put your display changing code within an if block that calls isDescriptionValid. (really, you shouldn't have a special function at all, and should just do the "if(description.value != "")" logic on its own)

Really this is just a very awkward design for validation and you should consider refactoring the whole thing.

Answer 5

Instead of reinventing the wheel, consider jQuery Validation plugin.

Answer 6

If you put an alert here:

                            ...

                            if (headlineValid(headline)){
                                alert('foo');
                                if(descriptionValid(description)){
                                    return true;
                                }
                            }

                            ...

Does the alert box show up? Validation methods not getting called and the form "just submitting" has usually meant that there's a JavaScript error somewhere in the validation code and it aborts there. If this alert box does not show up, move it upwards in your if hierarchy until it does - that would point you to where the error occurs. Of course, there are more sophisticated methods for debugging this stuff, such as Firebug. Good luck.

Answer 7

In validateForm() you have:

var description = document.getElementById("annonsera_des");

but you call:

descriptionValid(des)

Where is des? Instead, you should write:

if(descriptionValid(description)){

Passing des (which is undefined) is a problem.

Answer 8

I think the problem is in the if statements.

Q: Why isn't descriptionValid being called?
A: One of the if statement is returning false.
S: Break up the if statements.

Answer 9

It looks like everything on the rest of the form would have to validate before your function would even be called. Is it possible that something else is validating incorrectly, and therefore dropping out of your IF's, prior to reaching the method?

You could rewrite it to act as guard clauses, and either use a debugger as suggested or add an alert to the body of the IF for each of these to see which ones are hit.

if (! nameValid(name)){ return false;}
if (! telValid(tel)){ return false;}
....//continue for all etc.

//if none of these returned false just return true
return true;