don't validate hidden fields #189

Open
duduribeiro opened this Issue Aug 25, 2011 · 22 comments
@duduribeiro

Hi, i change the function:

elements: function() {
            var validator = this,
                rulesCache = {};

            // select all valid inputs inside the form (no submit or reset buttons)
            return $(this.currentForm)
            .find("input, select, textarea")
            .not(":submit, :reset, :image, [disabled]")

to:

            .not(":submit, :reset, :image, [disabled], :hidden")

to not validate hidden fields.. (in my case, sometimes a field will be hidden in the submit).

is this a valid change to the project?

@attiks

you can do it by setting the ignore option in the config like this:
validate_options.ignore = ':input:hidden';
$('myform').validate(validate_options)

@jzaefferer
Owner

How about making ":hidden" the default for the ignore option? That way it can be overriden, but doesn't have to be set all the time.

@hdragomir hdragomir added a commit to hdragomir/jquery-validation that referenced this issue Sep 4, 2011
@hdragomir hdragomir :hidden elements are now ignored by default
In response to #189
eef5109
@hdragomir hdragomir added a commit to hdragomir/jquery-validation that referenced this issue Sep 20, 2011
@hdragomir hdragomir Fixed #189 - :hidden elements are now ignored by default 695e7f9
@hdragomir hdragomir added a commit to hdragomir/jquery-validation that referenced this issue Sep 20, 2011
@hdragomir hdragomir :hidden elements are now ignored by default
In response to #189
f9d97cd
@wbotelhos

Hi,

This default option is not good, because hidden fields are valid field and can be submitted.
Hidden fields like the one the keep id value for edit a form must be validate to do the edit action not create.

On the other hand, validate hidden fields prevents the user defrauds the form when it is not showed like on jQuery Stepy on a Wizard form.

If you don't want validate the hidden field, simple, just do not put a rule in this field.

@duduribeiro

i think..
If you WANT validate the hidden field, put a rule...

@wbotelhos

Yes, of course my friend, read again,

"[..] just do NOT put a rule in this field"

If you don't put a rule on hidden field, it wont be validate.
But, add :hidden on ignore, even if you put a rule on it, it wont be validate, because you'll need take it of ignore.

On the other hand this default broke the code: #263

@hakunin

I just spent two hours trying to figure out why was my field ignored.

<input type="hidden" class="required">

I've put class="required" in it, why no validation?
Then I thought okay, maybe they don't validate hiddens.. so I did:

<input type="text" class="required hidden">

And still no validation.

I thought to myself: "why not validate those fields? if you don't want something validated, you simply don't put class="required" there.."

At that point I was breathing fire and went to the sourcecode, then to changelog and here.

Looking at #263, this looks like a rather ugly hack than a solution to the original problem.

@hdragomir

How about overriding the default ignore settings?

it could read ":hidden:not(.required)" and you're good.
Actually, this could be the new default.

@wbotelhos

Hi hakunin,

This "ugly" code is to avoid undefined on radio buttons validations trigged when the default :hidden is taked from the default exclusion. The original solution is to remove the :hidden from default exclusion, which will take you to the error in which I made the fix. If you had tried to fix the problem instead of just opening the issue, you would have seen it.

If you have an better solution for the error, do a push. (;

@mlynch mlynch added a commit to mlynch/jquery-validation that referenced this issue Mar 28, 2012
@hdragomir hdragomir Fixed #189 - :hidden elements are now ignored by default 6d364ec
@muldera

If I make a field required, I expect that because of this it will be validated. The visibility of that field should not come into question at all.

This change breaks a TON of code... anything in a collapsed accordion, anything in a tab that is not in focus. If I've taken the time to annotate a field to be validated, it should be validated.

@mlynch

@muldera it's not that simple. What do you do if it's invalid? The user will have no idea, so why don't you just show it by default?

If you really want the original behavior just set ignore: ""

@ctsears

I see this as a very bad idea. If a field is marked as required, it should be validated, whether hidden or not. Visibility and validation should have no default relationship to each other. There are countless places that hidden fields are validated. Think of all the required fields that are "hidden" inside tabs and accordion controls.

This was not well thought out. The burden should be on those who DON'T want to validate hidden fields.

@mlynch

Well this change was made 9 months ago and has had little in the way of complaints until now.

It's very easy to set ignore: "" to reset the default behavior. Try playing with the jQuery UI tabs demo and toggle the value of ignore to compare the behavior.

@wbotelhos
@mlynch

I think there are merits for keeping the default, but I also think the official tabs demo shows that ignore: ":hidden" doesn't make sense for all purposes (it feels really awkward in the demo).

I still don't see what the big deal is with setting ignore: "" in your own code. Either way it's not going to work for everyone.

@ctsears

Agreed. It's very easy to set the ignore option in every page that relies on hidden, required fields. I just think it's silly that we have to do it. It's just as easy to set this in every page that you don't want to validate hidden fields. You've just placed the burden on everyone else. You've tied validation to visibility by default. That just feels backward IMHO.

@bsingin64

In case my $1/50 matters... I agree that hidden elements SHOULD be validated. I am working a SPA, forms often have multiple tabs, along with logic which may hide / show fields based on other fields.

My vote: default don't exclude hidden elements.
Then again, 9 months worth of developers have been writing code with this rule in effect. It'd make this a breaking change for those apps possibly.

@jzaefferer
Owner

Thanks for the feedback @bsingin64. Based on the number of issues since 2006 and since the change nine months ago, I tend to think that the current default is the better choice. Neither approach is good enough, so unless we can come up with a solution that avoids the issue entirely, I'd like to stick with what we have now.

Better demos and documentation could certainly help address the issue, but that's just not possible with the current state of affairs. I need a sponsor for the project to focus on more then just addressing critical bugs.

@TheCrow1213

Is it still possible to allow validation of hidden fields by setting ignore: "" ?

    <script>
        $(document).ready(function() {
            $.validator.setDefaults({
                ignore: ""
            });

            $("form").validate({
                ignore: ""
            });
        });
    </script>

I have that tag in my page but hidden fields are still being skipped.

@staabm
Collaborator

@TheCrow1213 could you please open a new issue and link a small jsfiddle which shows a reduced example of the problem?

@TheCrow1213

@staabm Got it working. I'm not sure how... but it is. Thanks :)

@steveharrison
steveharrison commented Apr 18, 2016 edited

Realise that this change is handy for tabs/etc. but just wanted to echo the opinion that the default option should be to validate everything and then, if you particularly don't want to validate elements that aren't visible, you can add an ignore option.

I'd much rather debug an error caused by elements that are not visible requiring validation than wondering why some elements that are very clearly marked as required magically get skipped.

@staabm
Collaborator

@steveharrison we cannot change the selector to affect more elements as it does atm with breaking BC. We can discuss this for a possible 2.x version, but this will not be something happening in the near future

@staabm staabm reopened this Apr 18, 2016
@staabm staabm added this to the 2.0.0 milestone Apr 18, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment