Bug in contact.js breaking all validation logic

As an alternative, you could have an explicit “do not validate” class, rather than relying on finding nothing. Even better (because I find this a mild design flaw): separate out the options for validation of a value being a certain data type and it being a required field. It would be an additional checkbox in the admin interface and you would need to do some upgrade logic (or fall back to a sensible default) but it would follow two distinct validation paths:

1) is there a data type indicated –> (yes –> validate against regex; no –> ContenttypeValid = true)
2) should there be a value –> (yes –> good / nogood; no –> ContentPresentValid = true; don’t know –> data type set ==> assume yes –> good / nogood)

Then take ContentTypeValid && ContentPresentValid as a prerequisite for posting the form.

Thanks for picking it up. As I’ve stated, I have fixed it for myself, but (a) I am a JS novice and actual professionals probably should check my work and (b) maybe more people suffer from the same issue and thus fixing it from source is welcome.

It is this bit of code. I condensed it a bit for readability and added my own comments.

function checkElements( e ) {
	// reset validation var and send data
	send.validationError = false;
	send.datastring = 'ajax=true';
	//	Get in js added element (e.g. from reCAPTCHA)
	send.formElements = form.find('textarea, select, input[type=text], input[type=checkbox], input[type=hidden], input[type=email]');
	
	send.formElements.each(function(i) {
		var currentElement = $(this),
		surroundingElement = currentElement.parent(),
		value = currentElement.val(),
		name = currentElement.attr('name'),
	 	classes = currentElement.attr('class'),
	 	nomatch = true;
	 	if(currentElement.is(':checkbox'))
		 	{ if(currentElement.is(':checked')) { value = true; } else { value = ''; } }
			send.dataObj[name] = encodeURIComponent(value);
		 	// P: The following is a repeated pattern of testing a condition, setting a class on the surrounding element, either 'error' or 'valid'
		 	// P: Condition: "has a value" / "not empty"
		 	if(classes && classes.match(/is_empty/)) {
				if(value == '' || value == null) {
					surroundingElement.removeClass("valid error ajax_alert").addClass("error");
					send.validationError = true;
				} else {
					surroundingElement.removeClass("valid error ajax_alert").addClass("valid");
				}
				nomatch = false;
			}
			// P: Condition: type is e-mail
			if(classes && classes.match(/is_email/)) {
				if( ! value.match(/^[\w|\.|\-]+@\w[\w|\.|\-]*\.[a-zA-Z]{2,20}$/)) {
					surroundingElement.removeClass("valid error ajax_alert").addClass("error");
					send.validationError = true;
				} else {
					surroundingElement.removeClass("valid error ajax_alert").addClass("valid");
				}
				nomatch = false;
			}
			// P: Condition: e-mail with bells and whistles
			if(classes && classes.match(/is_ext_email/)) {
				//  also allowed would be: ! # $ % & ' * + - / = ? ^ _ 
 { | } ~
				if( ! value.match( /^[\w|\.|\-|ÄÖÜäöü]+@\w[\w|\.|\-|ÄÖÜäöü]*\.[a-zA-Z]{2,20}$/ ) ) {
					surroundingElement.removeClass("valid error ajax_alert").addClass("error");
					send.validationError = true;
				} else {
					surroundingElement.removeClass("valid error ajax_alert").addClass("valid");
				}
				nomatch = false;
			}
			// P: Condition: phone number
			if(classes && classes.match(/is_phone/)) {
				if( ! value.match(/^(\d|\s|\-|\/|\(|\)|\[|\]|e|x|t|ension|\.|\+|\_|\,|\:|\;){3,}$/)) {
					surroundingElement.removeClass("valid error ajax_alert").addClass("error");
					send.validationError = true;
				} else {
					surroundingElement.removeClass("valid error ajax_alert").addClass("valid");
				}
				nomatch = false;
			}
			// P: Condition: just numbers
			if(classes && classes.match(/is_number/)) {
				if( ! value.match( /^-?\s*(0|[1-9]\d*)([\.,]\d+)?$/ ) ) {
					surroundingElement.removeClass("valid error ajax_alert").addClass("error");
					send.validationError = true;
				} else {
					surroundingElement.removeClass("valid error ajax_alert").addClass("valid");
				}
				nomatch = false;
			}
			// P: Condition: positive number
			if(classes && classes.match(/is_positiv_number/)) {
				if(!($.isNumeric(value)) || value == "" || value < 0 ) {
					surroundingElement.removeClass("valid error ajax_alert").addClass("error");
					send.validationError = true;
				} else {
					surroundingElement.removeClass("valid error ajax_alert").addClass("valid");
				}
				nomatch = false;
			}
			// P: Handle the captcha
			if(classes && classes.match(/captcha/) && ! classes.match(/recaptcha/) ) {
				var verifier 	= form.find("#" + name + "_verifier").val(),
					lastVer		= verifier.charAt(verifier.length-1),
					finalVer	= verifier.charAt(lastVer);
				if(value != finalVer) {
					surroundingElement.removeClass("valid error ajax_alert").addClass("error");
					send.validationError = true;
				} else {
					surroundingElement.removeClass("valid error ajax_alert").addClass("valid");
				}
				nomatch = false;
			}
			// P: And HERE is my problem: if a form field has any value and it has no specific validation set, 'nomatch' will be true and this condition will hit. The return value will be "valid". However, if a form field has nothing filled in and is has no specific validation set, there is no remaining way for it to validate and hence that situation gets stuck without a 'valid' class.
			if(nomatch && value != '')
						{
							surroundingElement.removeClass("valid error ajax_alert").addClass("valid");
						}
				});

I found the effects as such: the form would not work and it would invalidate (red line around it, i.e. a class that matched the CSS for that red line) on all field without validation that were empty.

I changed the line
if(nomatch && value != '')
to
if(nomatch)

and it solved my problems.

I hope this clears up the core issue. My further suggestions for splitting out validation on content (“if there is a value, does it conform to the data type set, i.e. is a number a number and is a date a date”) and validation for mandatory presence (“does there need to be a value for the form to accept processing”) you can ignore.

• This reply was modified 4 years, 6 months ago by KdvPebbels.

I observed that the form fields in question turned red (which is imho different from ‘not green’) but more importantly: the form would not submit. After.my edit, that behaviour went away. That said, I had a rather difficult time getting the change to propagate because of JS minimising and other caching issues. Especially since I seem to be unique in this problem (where it should be widespread if I’m right) it may very well have been a transient error that got fixed by flushing the cache rather than my tweaking.

To be honest: it works right now and I would just as well leave it alone. I will report back here if the issue remains and/or reappears after an update (before i re-apply this manual tweak).