JavaScript & Events - Best Practice

Question

I once read that the following coding technique is considered bad:

<a HREF="page.htm" onClick="alert('Hello World')">text link</a>

Basically, having these event handlers (like onClick) within the HTML tag is the wrong approach to use. What is the reason for this? What is this "bad" technique called? What is the "proper" way to do this?

Answer 1

A couple reasons. It's essentially an alias to the DOM Level 1 interface of element.onclick, which only allows for one event listener.

Another reason is that it's simply philosophically wrong. It leads to poorly organized code: You're mixing a declarative language with a functional language which incorporates massive amounts of logic in its execution.

The proper way would be:

element.addEventListener('click', function() {
  console.log('hello world');
}, false); // required for old gecko

Or in IE:

element.attachEvent('onclick', function() {
  console.log('hello world');
});

or at the very least, use the DOM level 1 model:

element.onclick = function() {
  console.log('hello world');
};

(Also note, alert is bad practice, it will block execution of the entire page.)

Another reason would be, you don't get access to any kind of event object you would normally get as the first argument of the listener's callback. (You're essentially inside a callback when setting on[event] attributes, so I'm not sure if you can do arguments[0] or how different rendering engines implement it. It's very awkward, which is why it's best to bind events in JS-land.)

The final reason I can think of would be cross browser compatibility. You can't ever normalize an event interface for yourself if you're binding events in HTML.

Answer 2

This is an inline event handler attribute. (+1 chjj's answer for alternatives). It's generally considered ‘bad’ for a number of reasons:

• you're mixing small pieces of JavaScript syntax inline with HTML syntax:

  • when you have lots of these, especially when you have lots of elements that all contain essentially the same bit of code, it's harder to read and maintain the code;

  • you get nested-escaping horrors when you need to use special-to-HTML characters in your code:

eg.:

<a href="x.html" onclick="this.innerHTML= '&lt;em>I like &quot;fish &amp;amp; chips&quot;&lt;/em>';">

• properties of the target element and all ancestor nodes become variables, potentially hiding your own variables of the same name. See this question for background on this surprising and almost always unwanted behaviour;

  • this introduces spooky incompatibilities as browsers have different DOM properties;

  • and future browsers introducing new properties will potentially break your code.

Answer 3

Not sure of the official term, but it's not good practice because HTML should be pure markup, without mixing client side script directly into it.

Best practice is attaching the event in such way:

window.onload = function() {
    document.getElementById("MyLink").onclick = function() {
        alert('Hello World');
        return false;
    }
}

After giving the link ID MyLink for example.