Opened 7 years ago
Closed 7 years ago
#18223 closed defect (fixed)
[regression] dojox/validate/check marks field as invalid if the constraint function returns a boolean true
Reported by: | Cormac Flynn | Owned by: | dylan |
---|---|---|---|
Priority: | blocker | Milestone: | 1.10.1 |
Component: | Dojox | Version: | 1.10.0 |
Keywords: | Cc: | ||
Blocked By: | Blocking: |
Description
Since 1.10.0, a regression has been introduced into dojox/validate/check that causes the validate.check method to mark a field as invalid if the constraint returns a boolean true.
To reproduce, consider the following code
require(['dojo/dom', 'dojox/validate/check', 'dojox/validate/web'], function(dom, checkForm, baseValidate) { var profile = { required: ['UserEmail'], constraints: { UserEmail: [baseValidate.isEmailAddress, false, true] } }; var form = dom.byId('myForm'); var results = checkForm(form, profile); // Will *always* report the field UserEmail as being invalid console.log(results.getInvalid()); });
The offending code begins at line 282 of check.js (https://github.com/dojo/dojox/blob/master/validate/check.js#L282)
// if constraintResponse is false (backwards compatibility with last version) or if property isValid is false, return the invalid field name and/or the constraintResponse message if(!constraintResponse){ invalid[invalid.length] = elem.name; }else if(!constraintResponse.isValid){ invalid[invalid.length] = { field : elem.name, message : constraintResponse.message }; }
At this point, as constraintResponse is a boolean true, it passes into the else and marks the field as invalid.
Change History (6)
comment:1 Changed 7 years ago by
Milestone: | tbd → 1.10.1 |
---|---|
Owner: | set to dylan |
Priority: | undecided → blocker |
Status: | new → assigned |
comment:2 Changed 7 years ago by
Summary: | dojox/validate/check marks field as invalid if the constraint function returns a boolean true → [regression] dojox/validate/check marks field as invalid if the constraint function returns a boolean true |
---|
comment:3 Changed 7 years ago by
@redking, so I think I could update the second check here to be something like:
}else if(typeof constraintResponse !== "boolean" && !constraintResponse.isValid){
but I'm concerned that this will do a false pass if the required field is empty.
The logic in this method is pretty brittle, so I could use a sanity check.
comment:4 Changed 7 years ago by
Yeah that makes sense to me; the boolean true
would miss both branches
comment:5 Changed 7 years ago by
Pull request at https://github.com/dojo/dojox/pull/134 , I'll land this tomorrow unless I hear objections.
comment:6 Changed 7 years ago by
Resolution: | → fixed |
---|---|
Status: | assigned → closed |
Ok, this was caused by #15495 . Will look into it asap.