views:

3486

answers:

4

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

A: 

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);
}
Perspx
+2  A: 

I usually use something like:

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

this should work both in IE e Firefox.

Enreeco
This works perfectly, can you explain why setting it up as a "new function" (even though the function fnDisplay_Computers()" is already defined, solves the problem?
Nicholas Kreidberg
I think it's a problem of the data type involved and the reference to the "new Function" associated with the new onclick method...but I really don't know the exact answer, sorry!
Enreeco
+7  A: 

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. http://stackoverflow.com/questions/422784 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))
            );
        };
    };
}
bobince
EXCELLENT reply, wow did you cover all the bases. Fortunately the strings are all sanitized before alines[x] is ever even referenced and the incoming values are part of a defined array anyway so no need to worry there.
Nicholas Kreidberg
Is it safe to say the following: onclick is an event handler and NOT an attribute and therefore setting it as an attribute will not create the needed event handler for it to work at least under IE?
Nicholas Kreidberg
Yes. It's actually both a native-script event handler property (of type Function) *and* a document-level attribute (of type String), but because IE<8 doesn't understand how attributes work it tries to set the event handler to the String instead of a Function compiled from it, which doesn't work.
bobince
Excellent, I get it. Thank you very much for your input bobince.
Nicholas Kreidberg
A: 

I tihnk #3 protesteth too much. In a lot of situations I'm building a table dynamically and need to pass parameters to the callback function. It isn't a typesafe issue since my variable parm is an integer index to the table row in question. Here's code with both a variable and fixed parameter that seems to be cross-browser compliant:

for (i = 0; i < arrTableData.length; i++) {

eleTR = objTable.insertRow(i+1);

cell = eleTR.insertCell(0);
cell.width = "21";
var myElement = document.createElement('img');
myElement.setAttribute('src', 'images/button_down.gif');
myElement.setAttribute('alt', 'Move item down');
myElement.onclick = new Function('moveItem(' + i + ',0)');
cell.appendChild(myElement);
marty nickel