1

EDIT #2:

Made a JS Fiddle... http://jsfiddle.net/N2p6G/ (I hardcoded some stuff that I'm certain works correctly, but the problem is still there)

Original:

So, I have written tens of thousands of lines of javascript, and used code that look like this a hundreds of times and I don't understand what's going on.

blacklistitembutton.onclick = function() {
  console.log("clicked.");
}

The above code does not seem to be working... and I can't figure out why

In fact, I use the same method earlier in the same file... and it works fine!

settings.onclick = function() {
  settings_popup.toggle();
}

EDIT:

Might it have something to do with the fact that it's being executed in a for loop? Here is the code...

var blacklistButton = document.createElement('input');
blacklistButton.type = 'button';
blacklistButton.value = "Add Current Site to Blacklist";
blacklistButton.onclick = function() {
  console.log('blacklistButton clicked');
}

for (var i=0;i<blacklist.length;i++) {
  var blacklistitembutton = document.createElement('div');
  blacklistitembutton.type = 'button';
  blacklistitembutton.blacklistValue = blacklist[i];
  blacklistitembutton.value = "X";
  blacklistitembutton.onclick = function() {
    console.log("clicked.");
  }
}

Then both blacklistButton and all of the blacklistitembuttons are put into the document through element.appendChild (and they all show up successfully!)

The blacklistButton onclick fires just fine, and the blacklistitembutton onclick does not.

Improve this question
7
  • Can you make a jsFiddle document (jsfiddle.net) Commented Aug 2, 2012 at 18:28
  • what browser are you running this on? also, can you provide more code for your example Commented Aug 2, 2012 at 18:29
  • 1
    what's blacklistitembutton and how you define it? Commented Aug 2, 2012 at 18:35
  • I'll edit and provide a little more information on defining blacklist item button Commented Aug 2, 2012 at 18:39
  • Are you making sure that the dom is ready? Commented Aug 2, 2012 at 18:43

4 Answers 4

Reset to default
2 
document.addEventListener('click', function(){
    console.log('clicked');
}, false);

Edit:

Here is a re-write of your code in a fiddle: http://jsfiddle.net/N2p6G/1/

There are a lot of things in your code that worry me. Hopefully from my re-write you can see there are better ways to handle some things.

1) I'm not sure why you are using document.write() at the beginning. That has very little purpose.

2) You are modifying the DOM way too much. Some of the DOM elements you are creating in code are better-served as just being target locations in html. Only the dynamically-created input button elements need to be done in javascript. Remember, modifying the DOM should be done as little as possible.

3) Don't assign events using the onclick, onsubmit, onhover, etc syntax. Events should only be bound to DOM elements using addEventListener. The other benefit of doing it the proper way is that you can assign multiple events of the same type, if need be, to the same element. Also, with some extra state code that I haven't included, you can selectively remove particular events later if you need to.

4) There was a debate several years ago about whether using innerHTML and string templates was faster/better than using DOM creation methods. For a while, the best solution was to use documentFragments and a combination of the two methods. These days, it doesn't really matter anymore since all browsers are pretty damn fast, so for simplicity's sake is good to just go with innerHTML.

This also goes back to the rule of "don't touch the DOM too much". If you look at my code, you can see that I'm assembling the final html simply as an array of elements that gets joined as a single string at the end. Its then rendered to the DOM with a single innerHTML statement. I'm only touching the DOM one time, instead of multiple times.

5) The last bit goes into events again. At the beginning and end of the code you can see where and how I've added the events for the DOM elements. Indeed, the addEventListener at the beginning could be moved to the end to group all the event declarations together, but it doesn't really matter. I left it at the top to help you understand what's going on better.

Hope this helps.

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

We need to see a full example of your code. Go to jsfiddle.net and set one up.
Yeah, the code I put in the jsfiddle is pretty different than my actual code. In my actual code I have functions that I use to append things such that everything is put together in the javascript and then appended once.
It didn't really... help... because you never answered my question... but thanks anyway I guess. As a rule of thumb "don't say more than you have to", keep it simple ;)
0

For unlimited event bindings, either use addEventListener or attachEvent method. You cannot add more than one event of the same type using that traditional method.

Improve this answer

1 Comment

@zconnelly13 jsfiddle.net/M8mTn Here is a demo, go through it and make the best out of yourself!
0

I don't know if it's a typo in what you put here, but in the loop you are creating a "div" and then assigning it a type of "button". Does that work or is it throwing an error? If it is then that explains why the event handler is not getting the function. Try making it an "input" and see if it now works.

Improve this answer

1 Comment

yeah, it was a typo, that I didn't make in the js fiddle jsfiddle.net/N2p6G
0

Fixed it!

blacklistitem.innerHTML += blacklist[i];

^ was messing it up, at this point in the code blacklistitem is still a javascript item, not yet appended to its to-be parent element in the document

So I just stuck blacklist[i] into a span tag and appended as a child and now it works fine :)

Improve this answer

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.