2

I'm working with a JavaScript routine I didn't write. It is called from a text box's onkeydown attribute to prevent unwanted keystrokes.

The first argument is apparently not used. The second argument is a list of characters that should be allowed.

function RestrictChars(evt, chars) {
    var key;
    var keychar;

    if (window.event)
        key = window.event.keyCode;
    else if (e)
        key = e.which;
    else
        return true;

    keychar = String.fromCharCode(key);

    if ((key == null) || (key == 0) || (key == 8) ||
        (key == 9) || (key == 13) || (key == 27))
        // Control key
        return true;
    else if (((chars).indexOf(keychar) > -1))
        return true;
    else
        return false;
}

This seems to work for alpha-numeric characters. However, characters such as . and / cause this function to return false, even when these characters are included in the chars parameter. For example, if the . key is pressed, key is set to 190, and keychar gets set to the "3/4" character.

Can anyone see how this was meant to work and/or why it doesn't? I don't know enough about JavaScript to see what it's trying to do.

Jonathan Wood
  • 65,341
  • 71
  • 269
  • 466
  • 1
    You might find this key code testing utility helpful for figuring out which events return what key codes for various keys: http://www.west-wind.com/WestwindWebToolkit/samples/Ajax/html5andCss3/keycodechecker.aspx – Rick Strahl Dec 14 '11 at 10:14

2 Answers2

5

Two things are wrong with that: first, if you're analysing what character has been typed, you need to use the keypress event instead of keydown because that is the only event that tells you anything reliable about the actual character typed. For (a lot) more detail about about this and JavaScript key events in general, see http://unixpapa.com/js/key.html. Second, there are references to a variable called e which doesn't (but should) correspond with the evt parameter.

Here's a rewrite, assuming you have a variable called textBox that refers to the text input element.

jsFiddle: http://jsfiddle.net/9DZwL/

Code:

function isKeypressCharValid(e, chars) {
    e = e || window.event;

    // Allow delete, tab, enter and escape keys through
    if (/^(8|9|13|27)$/.test("" + e.keyCode)) {
        return true;
    }

    var charCode = (typeof e.which == "number") ? e.which : e.keyCode;
    var charTyped = String.fromCharCode(charCode);
    return chars.indexOf(charTyped) > -1;
}

textBox.onkeypress = function(evt) {
    if (!isKeypressCharValid(evt, "abc123")) {
        return false;
    }
};
Tim Down
  • 318,141
  • 75
  • 454
  • 536
  • Thanks. That seems to work. I'll read up on this but perhaps you could answer a couple of quick questions. 1) How come this doesn't prevent control characters such as backspace? 2) What the heck is `||` doing? (With my background, `||` is logical OR but that doesn't seem to make sense here.) – Jonathan Wood Jun 25 '11 at 00:06
  • @Jonathan: I just added a bit to deal with control characters. `||` in JavaScript returns the first operand if it is "truthy" (i.e. coerces to Boolean `true`) and the second operand otherwise, so can be used as a shortcut in conditional assignment. – Tim Down Jun 25 '11 at 00:11
  • @pst: JavaScript key events are a mess. I suggest reading the page linked to in my answer (vastly superior to the quirksmode page, IMO) or my shorter summary here: http://stackoverflow.com/questions/4285627/javascript-keycode-vs-charcode-utter-confusion/4285801#4285801 and then see if you think my answer's wrong. – Tim Down Jun 25 '11 at 00:14
  • @Ken: I outlined two reasons why it doesn't work: first, it's the wrong event, and second, there's an incorrect variable. Admittedly I didn't expand on what the code is trying to do, but I provided a link to a resource that explains in great detail exactly how JavaScript key events do work. – Tim Down Jun 25 '11 at 00:18
  • @Jonathah Wood: `||` is indeed logical OR. The code in the first `if` branch says "if key is equal to null OR key is equal to 0 or". As for your question about backspace, it's specifically part of that test with `key == 8` (which is backspace; `9` is tab, `13` is enter (or return), and `27` is escape. – Ken White Jun 25 '11 at 00:20
  • @Ken: I disagree overall, but I think my answer could have been clearer so I have now moved some bits around and expanded it a little. Would you consider it a reasonable answer now? – Tim Down Jun 25 '11 at 00:25
  • @Tim: Much better, IMO. +1 from me. – Ken White Jun 25 '11 at 00:31
  • @Ken: In C, C++, and C++, logical OR always resolves to a Boolean value. Here, it is resolving to a non-Boolean value. So it's different in that respect. Note that Tim's original code did not test for control characters, and yet the code seemed to allow them, thus my question. – Jonathan Wood Jun 25 '11 at 00:32
  • @Jonathan: It's resolving to a Boolean - true if the key matches one of the test conditions, false if not. This is a chained test - see my answer. – Ken White Jun 25 '11 at 00:34
  • @Ken: I'm talking about the first line in @Tim's code. The OR operators in your code, I understand. – Jonathan Wood Jun 25 '11 at 00:47
  • @Jonathan: I gave a fairly terse explanation of my use of `||` in a comment above. Mozilla's docs go into a little more detail: https://developer.mozilla.org/en/JavaScript/Reference/Operators/Logical_Operators – Tim Down Jun 25 '11 at 01:02
  • @Jonathan: Sorry - thought you were referring to the `||` in your original code. – Ken White Jun 25 '11 at 01:14
  • @Tim: Thanks for the updated code; however your original code (without the code that checks for control characters) appears to correctly allow control characters. This has me confused. It's as though `onkeypress()` is only executed for printable characters. But I couldn't find that documented anywhere. – Jonathan Wood Jun 25 '11 at 03:57
  • @Jonathan: The `keypress` event is primarily for detecting keystrokes that produce printable characters, and all major browsers support it for that. However, some browsers fire `keypress` events for non-printable keys as well, and it's this behaviour that varies a lot. http://unixpapa.com/js/key.html has a lot of detail on this. – Tim Down Jun 25 '11 at 10:48
  • @pst: Your bold statement that you think my answer is incorrect is bothering me. I'm certain you're wrong and your comment is loudly undermining the authority of my answer, making it less useful for people coming to it in future. – Tim Down Jul 19 '11 at 15:45
  • @Tim Down Fair enough. I shouldn't have bolded it. –  Jul 20 '11 at 21:25
1

I'm not a JS person, either, but... I can explain how it's supposed to work; I don't know why you're getting the values you are for the keys you mentioned, however.

keychar = String.fromCharCode(key);

This checks to see if the key is a printable character (letter, punctuation mark, etc.)

if ((key == null) || (key == 0) || (key == 8) ||
    (key == 9) || (key == 13) || (key == 27))
    // Control key

The above checks to see if the key is null OR (||)` 0 or 8 (backspace) or 9 (tab) or 13 (0x0D, or ENTER) or 27 (0x1B or ESCAPE) - it's exactly the Boolean result you'd expect: IF <thiscondition> or <thatcondition> or <anothercondition> or ...

else if (((chars).indexOf(keychar) > -1))

This checks to see if the keychar is in the string of characters passed as the chars parameter

Ken White
  • 123,280
  • 14
  • 225
  • 444