Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
Core: Validate inputs with duplicate name / name[] arrays, instead of just the first item#2498
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:master
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
| var type = element.attr( "type" ); | ||
| if ( type === 'checkbox' || type === 'radio' ) { | ||
| // only check the first checkbox/radio input ( https://github.com/jquery-validation/jquery-validation/pull/2431#issuecomment-1172835268 ) | ||
| if ( element.name in checkedCache ) { | ||
| continue; | ||
| } | ||
| checkedCache[ element.name ] = true; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
This is not clear why we check only first checkbox?
What about the cases when we ask user to check boxes which labels does not havea letter?
| // Select only the first element for each name, and only those with rules specified | ||
| if ( name in rulesCache || !validator.objectLength( $( this ).rules() ) ) { | ||
| if ( name in rulesCache ) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Actually this is correct, that we do not allow elements with the same name. I believe this is by RFC. Though I could be wrong, because I did not work with checkbox/radio a lot.
KES777 commentedJan 26, 2025 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
@bytestream I am not familiar with checkbox/radio input types, but it looks like they try to resolve a different problem. |
Description
Replaces#2000 and#2431 with simpler logic, and I believe that .elements() should return all of the eligible elements.
Previously, if you had 2+ inputs using name arrays, such as:
It would only hit validation on the first input in the array, allowing all the remaining inputs with the same name to remain empty.
Technically, I'm pretty sure the following, without using [] syntax would have the same problem:
I'm not familiar with the testing framework, but please review the idea behind this PR first, and if you're confident that it will be merged if tests are included, I'll dig deeper into testing.