views:

371

answers:

9

So I am writing a registration form and I need the display name to be only numbers, letters and underscores. Have a look at my code and tell me what I'm doing wrong.

<form method="post" action="/" onsubmit="return check_form()">
    <input type="text" id="display-name" name="display-name" maxlength="255" />
    <input type="submit" />
</form>
<script type="text/javascript">
<!--
    var name_regex = /^([a-zA-Z0-9_])+/

    function check_form()
    {
     if (!name_regex.test(document.forms[0].elements[0].value))
     {
      document.forms[0].elements[0].focus()
      alert("Your display name may only contain letters, numbers and underscores")
      return false
     }
    }
-->
</script>

It's obviously been trimmed down to not include anything not related to the problem but even this snippet doesn't work.

+4  A: 

My regexp would go along the lines of: /^[a-zA-Z0-9_]+$/

edit: I think it's the lack of a line end ($) that makes it fail.

Annan
A: 

By 'not working' I take it you mean it is letting invalid entries through (rather than not letting valid entries through).

As Annan has said, this would probably be due to the lack of the '$' symbol at the end of the expression, as currently it only requires a single valid character at the start of the value, and the rest can be anything.

samjudson
+10  A: 

Your regex

/^([a-zA-Z0-9_])+/

looks for

  1. Start of string(check), followed by
  2. 1 or more letters, numbers, or underscore (check)

and then whatever comes after it doesn't matter. This regex will match anything at all so long as it begins with a letter, number, or underscore

If you put a $ at the end, then it will work - $ matches 'end of string', so the only way it can match is if there are only numbers, letters, and underscores between the start and end of the string.

/^([a-zA-Z0-9_])+$/

Secondly, I'd suggest using document.getElementById('display-name').value instead of document.forms as it won't break if you rearrange the HTML, and is more 'the commonly accepted standard of what to do'

Orion Edwards
\w is a shorthand for letter, digits and underscores. I would simplify it to /^\w+$/
Juan Mendes
A: 

I see the snippet is missing semicolons (which is what defines where the code row ends).

Andy
A: 

What does "doesn't work" mean? Does it reject valid display names? Does it accept invalid display names? Which ones?

Per @Annan, leaving off the $ would make the regexp accept invalid display names like abc123!@#.

If the code is rejecting valid display names, it may be because the parentheses are being matched literally instead of denoting a group (I'm not sure of the quoting convention in JS).

Chris Conway
A: 

I tested your script and meddled with the javascript. This seem to work:

<form method="post" action="/" onsubmit="return check_form()">
    <input type="text" id="display-name" name="display-name" maxlength="255" />
    <input type="submit" />
</form>
<script type="text/javascript">
<!--
    var name_regex = /^([a-zA-Z0-9_])+$/;

    function check_form()
    {
        if (!name_regex.test(document.forms[0].elements[0].value))
        {
                document.forms[0].elements[0].focus();
                alert("Your display name may only contain letters, numbers and underscores");
                return false;
        }
    }
-->
</script>
Andy
A: 

Sorry guys I should have been more specific. Whenever I added spaces the values were still being accepted. The dollar sign ($) did the trick!

Andrew G. Johnson
A: 

A simpler way to write it still would be

var name_regex = /^([a-z0-9_])+$/i;
Swish
A: 

Even simpler:

var name_regex = /^\w+$/;
travis