javascriptdom-eventsaddeventlistenervbulletin

Javascript eventhandler always being added to last object in array


I am trying to perform two tasks on a "toggle collapse" button (which is actually an image):

  1. If a section of the page is loaded collapsed (from cookie) then add the class "collapsed" to an ancestor tag.
  2. Add an event handler to toggle the presence of the "collapsed" class when the button is clicked.

To do so, I have the following code:

function toggleCollapsedPlugin(sender, collapseState) {
    // do something
    // expects 1 OR 2 arguments
}

function initCollapsedPlugin() {
    var forumcats = document.getElementById('forums').getElementsByClassName('forumbit_nopost L1');
    var button;
    var buttonParent;
    for (var i = 0; i < forumcats.length; i++) {
        button = forumcats[i].getElementsByTagName('a')[1].getElementsByTagName('img')[0]; // will always exist
        buttonParent = button.parentNode.id; // will never be empty
        if (button.src.indexOf('_collapsed') >= 0) {
            toggleCollapsedPlugin(button.parentNode.id, true);
        }
        button.addEventListener('click', function () { toggleCollapsedPlugin(buttonParent); }, false);
    }
}

As I step through initCollapsedPlugin() for the first item in the Chrome DevTools, everything appears to be fine. When I later click one of the images to call toggleCollapsedPlugin(sender), sender is always equal to buttonParent of the last item in forumcats. Can someone explain why and how to fix this?

(If it helps anyone provide an answer, this is the vBulletin4 software.)


Solution

  • This is a classic 'pitfall' of JavaScript closures. Basically, your buttonParent variable is being overwritten in every iteration and the anonymous function you're passing to addEventListener references but does not copy this variable.

    You'll probably need something similar to:

    button.addEventListener('click', (function (parent) {
        return function() {
            toggleCollapsedPlugin(parent);
        };
    })(buttonParent), false);
    

    For a more detailed explanation, see How do JavaScript closures work?.

    In your case however, you can simply get rid of the closed variable altogether and find the button's parent when the anonymous function is called. This works because this inside of the callback function refers to the element the event was registered on, being the element references by button in that iteration.

    button.addEventListener('click', function() {
        return toggleCollapsedPlugin(this.parentNode.id);
    }, false);