The form does not work correctly when sent

Question

I wrote the code for a form validation.

Should work like this:

It checks (allLetter (uName)) and if it's true, then validate the next input.

If any validation is false then it should return false.

My problem is that if both validations are true, then everything is exactly false and the form is not sent.

If I set true in formValidation (), if at least one check false, the form should not be sent.

<form name='registration' method="POST" onSubmit="return formValidation();">
    <label for="userName">Name:</label>
    <input type="text" name="userName" size="20" />
    <label for="userPhone">Phone:</label>
    <input type="text" name="userPhone" size="20" />
    <input type="submit" name="submit" value="Submit" />
</form>

function formValidation() {
  var uName = document.registration.userName;
  var uPhone = document.registration.userPhone;
    if(allLetter(uName)) {
      if(phone(uPhone)) {}
    }
    return false;
  }


function phone(uPhone){ 
  var digts = /^[0-9]+$/;
  if(uPhone.value.match(digts)){
    return true;
  } else {
    alert('Phone must have only digits');
    uPhone.focus();
    return false;
  }
}

function allLetter(uName) { 
  var letters = /^[A-Za-z]+$/;
  if(uName.value.match(letters)) {
    return true;
  }else{
    alert('Username must have alphabet characters only');
    uName.focus();
    return false;
  }
}

Hopefully this is a cosmetic layer in front of some server-side validation, as you should never validate only on the client. But as long as you are, why not just use HTML5 `pattern` attributes? Much easier for basic validation. — Mitya, Apr 16 '18 at 14:20
Validation is only through JS without `pattern`, I'm also checking the server side. — Michael Podlevskykh, Apr 16 '18 at 14:23

Scott Marcus · Accepted Answer · 2018-04-16T14:55:09.313

First, you are using a 20+ year old way to gain references to your elements (document.form.formElementNameAttributeValue) and, while this still works for legacy reasons, it doesn't follow the standard Document Object Model (DOM) API.

Next, you've broken up your validation tests into different methods (and that's certainly not a bad idea for reusability), but in this case is is adding a ton of code that you just don't need. I've always found it's best to start simple and get the code working, then refactor it.

You're also not using the <label> elements correctly.

One other point, your form is set to send its data via a POST request. POST should only be used when you are changing the state of the server (i.e. you are adding, editing or deleting some data on the server). If that's what your form does, you'r fine. But, if not, you should be using a GET request.

Lastly, you are also using a 20+ year old technique for setting up event handlers using inline HTML event attributes (onsubmit), which should no longer be used for many reasons. Additionally, when using this technique, you have to use return false from your validation function and then return in front of the validation function name in the attribute to cancel the event instead of just using event.preventDefault().

So, here is a modern, standards-based approach to your validation:

// Get references to the elements you'll be working with using the DOM API
var frm = document.querySelector("form[name='registration']");
var user = document.getElementById("userName");
var phone = document.getElementById("userPhone");

// Set up event handlers in JavaScript, not with HTML attributes
frm.addEventListener("submit", formValidation);

// Validation function will automatically be passed a reference
// the to event it's associated with (the submit event in this case).
// As you can see, the function is prepared to recieve that argument
// with the "event" parameter.
function formValidation(event) {

  var letters = /^[A-Za-z]+$/;
  var digts = /^[0-9]+$/;
  
  // This will not only be used to show any errors, but we'll also use
  // it to know if there were any errors.
  var errorMessage = "";
  
  // Validate the user name
  if(user.value.match(letters)) {

     // We've already validated the user name, so all we need to
     // know now is if the phone is NOT valid. By prepending a !
     // to the test, we reverse the logic and are now testing to 
     // see if the phone does NOT match the regular expression
     if(!phone.value.match(digts)) {
       // Invalid phone number
       errorMessage = "Phone must have only digits";
       phone.focus();
     }
     
  } else {
    // Invalid user name
    errorMessage = "Username must have alphabet characters only";
    user.focus();
  }

  // If there is an error message, we've got a validation issue
  if(errorMessage !== ""){
    alert(errorMessage);
    event.preventDefault(); // Stop the form submission
  }

}

<!-- 20 is the default size for input elements, but if you do 
     want to change it do it via CSS, not HTML attributes -->
<form name='registration' method="POST">
    <!-- The for attribute of a label must be equal to the id 
         attribute of some other element, not the name attribute  -->
    <label for="userName">Name:</label>
    <input type="text" name="userName" id="userName">
    <label for="userPhone">Phone:</label>
    <input type="text" name="userPhone" id="userPhone">
    <input type="submit" name="submit" value="Submit">
</form>

thank you for `20 years`, I honestly didn't know. I copied this code on the site and did not know what is it, but I guessed and decided not to change it. @ScottMarcus — Michael Podlevskykh, Apr 16 '18 at 14:51
@МишаПодлевских Unfortunately, these old techniques get copied and pasted by new coders that don't know/understand the modern best-practices of the web and so they just won't go away, like they should. I would guess that the person who made the site you copied it from, copied it from somewhere else themselves. — Scott Marcus, Apr 16 '18 at 14:52

The form does not work correctly when sent

1 Answers1