setAttribute, onClick and cross browser compatibility

Question

I have read a number of posts about this but none with any solid answer. Here is my code:

// button creation
onew = document.createElement('input');
onew.setAttribute("type", "button");
onew.setAttribute("value", "hosts");
onew.onclick = function(){fnDisplay_Computers("'" + alines[i] + "'"); }; // ie
onew.setAttribute("onclick", "fnDisplay_Computers('" + alines[i] + "')"); // mozilla
odiv.appendChild(onew);

Now, the setAttribute() method (with the mozilla comment) works fine in mozilla but only if it comes AFTER the line above it. So in other words it seems to just default to whichever gets set last. The .onclick method (with the ie comment) does not work in either case, am I using it incorrectly?

Either way I can't find a way to make this work at all in IE, let alone in both. I did change the function call when using the .onclick method and it worked fine using just a simple call to an alert function which is why I believe my syntax is incorrect.

Long story short, I can't get the onclick parameter to work consistently between IE/Mozilla.

-- Nicholas

Solution 1

I usually use something like:

onew.onclick = new Function("fnDisplay_Computers('" + alines[i] + "')");

this should work both in IE e Firefox.

Solution 2

onew.setAttribute("type", "button");

Never use setAttribute on HTML documents. IE gets it badly wrong in many cases, and the DOM-HTML properties are shorter, faster and easier to read:

onew.type= 'button';

onew.onclick = function(){fnDisplay_Computers("'" + alines[i] + "'"); }; // ie

What is ‘alines’? Why are you converting it to a string and surrounding it with single quotes? It looks like you are trying to do something heinous involving evaluating code in a string (which is what you're doing below in the ‘onew.setAttribute’ version). Evaluating JavaScript code in strings is almost always the Wrong Thing; avoid it like the plague. In the above case, IE should do the same as Firefox: it shouldn't work.

If ‘alines[i]’ is a string, I guess what you're trying to do is make it remember that string by constructing a code string that will evaluate in JavaScript to the original string. But:

"'" + alines[i] + "'"

is insufficient. What happens if ‘alines[i]’ has an apostrophe in, or a backslash?

'O'Reilly'

you've got a syntax error and possible security hole. Now, you could do something laborious and annoying like:

"'" + alines[i].split('\\').join('\\\\').split("'").join("\\'") + "'"

to try to escape the string, but it's ugly and won't work for other datatypes. You could even ask JavaScript to do it for you:

uneval(alines[i])

But not all objects can even be converted to evaluatable JavaScript source strings; basically the entire approach is doomed to failure.

The normal thing to do if you just want to have the onclick callback call a function with a parameter is to write the code in the straightforward way:

onew.onclick= function() {
    fnDisplay_Computers(alines[i]);
};

Generally this will work and is what you want. There is, however, a slight wrinkle which you may have hit here, which could be what is confusing you into considering the wacky approach with the strings.

Namely, if ‘i’ in this case is the variable of an enclosing ‘for’ loop, the reference to ‘alines[i]’ won't do what you think it does. The ‘i’ will be accessed by the callback function when the click happens — which is after the loop has finished. At this point the ‘i’ variable will be left with whatever value it had at the end of the loop, so ‘alines[i]’ will always be the last element of ‘alines’, regardless of which ‘onew’ was clicked.

(See eg. How to fix closure problem in ActionScript 3 (AS3) for some discussion of this. It's one of the biggest causes of confusion with closures in both JavaScript and Python, and should really be fixed at a language level some day.)

You can get around the loop problem by encapsulating the closure in its own function, like this:

function callbackWithArgs(f, args) {
    return function() { f.apply(window, args); }
}

// ...

onew.onclick= callbackWithArgs(fnDisplay_Computers, [alines[i]]);

And in a later version of JavaScript, you'll be able to say simply:

onew.onclick= fnDisplay_Computers.bind(window, alines[i]);

If you would like to be able to use ‘Function.bind()’ in browsers today, you can get an implementation from the Prototype framework, or just use:

if (!('bind' in Function.prototype)) {
    Function.prototype.bind= function(owner) {
        var that= this;
        var args= Array.prototype.slice.call(arguments, 1);
        return function() {
            return that.apply(owner,
                args.length===0? arguments : arguments.length===0? args :
                args.concat(Array.prototype.slice.call(arguments, 0))
            );
        };
    };
}

Solution 3

Use the addEventListener() function with "click" for the type argument for Mozilla-based browsers, and attachEvent() function with "onclick" as the sEvent argument for IE; I find it best to use a try/catch statement, for example:

try {
  onew.attachEvent("onclick", //For IE
                   function(){fnDisplay_Computers("'" + alines[i] + "'"); });
}
catch(e) {
  onew.addEventListener("click", //For Mozilla-based browsers
                   function(){fnDisplay_Computers("'" + alines[i] + "'"); },
                   false);
}