JavaScript stack overflow

Here's one that some of you may have seen before, but I thought I'd post it to save some time to those who didn't.

Today we were trying to debug some client-side code and we needed to quickly wire the click event of a button. So we did this, without thinking too hard:

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN"
    "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
    <title>Stack overflow</title>
    <script type="text/javascript">
    function onclick() {
        // do stuff
    }
    </script>
</head>
<body>
    <div>
        <input type="button" value="Click me" onclick="onclick();" />
    </div>
</body>
</html>

Can you spot the stack overflow? We were so focused on the "do stuff" part that we didn't see it at first, thinking it came from there. But of course, it just comes from onclick="onclick();". When JavaScript resolves the name "onclick", it looks first on the current context before it goes up the scope chain and finds our global function (which it never does). The current context here is the input element, which happens to have an "onclick" attribute, which executes "onclick", etc. Crash.

So the valuable lesson we've learned here is to adopt a naming convention for our event handlers that can't conflict with the name of the event. I usually choose elementIdOnEventName. For example here if the input's id is button1, we'd go with button1OnClick.

6 Comments

  • I especially like how IE handles stack overflow. *POP!* and IE is gone...
    Stack overflow is actually a faster way of closing down IE than clicking the close button :-)

  • Errr, Morten, which version of IE are you using? I'm getting the error message in a dialog and it doesn't crash.

  • I prefer using elementID_EventName. It is close to the C#/VB convention and makes it more understandable for people viewing my client-side code that are not used to code javascript.

    Cheers,
    Erjan

  • Good point with this overflow by wrong name.

    PS: you need to do something with the code layout.
    I had to copy the code into notepad2 to see what the onclick event was.

  • that is quite obtrusive! try the jquery way

  • Matt, you're not paying attention. In ASP.NET Ajax, you attach events using Sys.UI.DomEvent.addHandler or $addHandler. This topic of this post is not how you should attach DOM events though. It's how when throwing together some quick test code (and in test code, I personally feel free to use obtrusive event handlers as I'm the only one who should care and it's way quicker to set-up, but feel free to do however you prefer), you can shoot yourself in the foot and spend some time figuring out what's happening.

Comments have been disabled for this content.