Opened 5 years ago

Closed 5 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 5 years ago by dylan

Milestone: tbd1.10.1
Owner: set to dylan
Priority: undecidedblocker
Status: newassigned

Ok, this was caused by #15495 . Will look into it asap.

comment:2 Changed 5 years ago by bill

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 5 years ago by dylan

@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 5 years ago by Cormac Flynn

Yeah that makes sense to me; the boolean true would miss both branches

comment:5 Changed 5 years ago by dylan

Pull request at https://github.com/dojo/dojox/pull/134 , I'll land this tomorrow unless I hear objections.

comment:6 Changed 5 years ago by dylans <dylan@…>

Resolution: fixed
Status: assignedclosed

In e39b936a2d88524d32763ee4855c4793984f1af4/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 
Note: See TracTickets for help on using tickets.