CSS Menu Rollovers not working in IE!

[gp_330]

Hi guys, I am using a drop down menu script from AListApart, which works by using CSS to convert an unordered list into drop downs. The code I have works in Firefox perfectly (of course), but of course IE only has one set of drop downs working.

I know that the problem is being caused by the javascript code, but I can't seem to figure out why it isn't working, and I was wondering if anyone in the forums could shine some light on my problem. My code looks like this:

startList = function() {
if (document.all&&document.getElementById) {
navRoot = document.getElementById("menu1");
for (i=0; i<navRoot.childNodes.length; i++)
{
node = navRoot.childNodes[i];
if (node.nodeName=="LI")
{
node.onmouseover=function() {this.className+=" over";}
node.onmouseout=function() {this.className=this.className.replace(" over", "");}
}
}
}
}

startList2 = function() {
if (document.all&&document.getElementById) {
navRoot2 = document.getElementById("menu2");
for (i=0; i<navRoot2.childNodes.length; i++)
{
node2 = navRoot2.childNodes[i];
if (node2.nodeName=="LI")
{
node2.onmouseover=function() {this.className+=" over";}
node2.onmouseout=function() {this.className=this.className.replace(" over", "");}
}
}
}
}

window.onload=startList;startList2


I am sure that the problem has got to do with some kind of naming convention problem, but I can't seem to see what could be causing it! Any help would be really appreciated.

Thanks
GP

[mwinter]

The code I have works in Firefox perfectly (of course),

That's because Firefox doesn't need the code.

but of course IE only has one set of drop downs working.

Your assignment to the onload property doesn't work the way you seem to think it should. More on that later.

startList = function() {
if (document.all&&document.getElementById) {
	navRoot = document.getElementById("menu1");
	for (i=0; i<navRoot.childNodes.length; i++) 
	{
		node = navRoot.childNodes[i];
		if (node.nodeName=="LI") 
			{
			node.onmouseover=function() {this.className+=" over";}
			  node.onmouseout=function() {this.className=this.className.replace(" over", "");}
			}
		}
	}
}

If this is what the original code looks like, it's very badly written.

You don't need to duplicate the functions, by the way. The only thing that differs between them is the argument to the getElementById method, so add a parameter that specifies that value.

var initialiseMenu = function() {
    var menus = [];

    function setClass() {
        if (!/(^|\s)over(\s|$)/.test(this.className))
            this.className += ' over';
    }
    function removeClass() {
        this.className = this.className.replace(/(^|\s)over(\s|$)/, ' ');
    }
    window.attachEvent('onunload', function() {
        for (var i = 0; i < menus.length; ++i) {
            menus[i].detachEvent('onmouseover', setClass);
            menus[i].detachEvent('onmouseout', removeClass);
        }
        menus.length = 0;
        window.detachEvent('onunload', arguments.callee);
    });

    return function(id) {
        var menu = document.getElementById(id),
            nodes = menu.childNodes;

        for (var index = 0, length = nodes.length, node; index < length; ++index) {
            node = nodes[index];
            if (node.tagName == 'LI') {
                node.attachEvent('onmouseover', setClass);
                node.attachEvent('onmouseout', removeClass);
            }
        }
        menus[menus.length] = menu;
    };
}();

window.attachEvent('onload', function() {
    initialiseMenu('menu1');
    initialiseMenu('menu2');

    window.detachEvent('onload', arguments.callee);
});

You should be aware of two things. The first is that this isn't tested: you didn't provide a link to your code. The second is that the above isn't exactly an example of good practice. It makes a heap of assumptions, but that's because you should place it in an external file and include it with:

<!--[if IE]><script type="text/javascript" src="/path/to/file"></script><![endif]-->

setting the src attribute appropriately. Only IE versions 5+ will see this code, so the assumptions that would break the code in other browsers won't matter here.

In case you're wondering why the code is larger, much of it is structured to allow the event listeners to be removed to try and avoid IE's notorious memory leak (which the original code will invoke). If you used this menu with the original code in several documents, the leak would grow as the user moved from document to document, and that clearly isn't a good thing.

window.onload=startList;startList2

The problem here is that there are two distinct statements. It could have just as easily been written as:

window.onload = startList;
startList2;

You can only assign one function to the onload property, so that function must invoke the other two:

window.onload = function() {
    startList();
    startList2();
};

If you have problems with what I've proposed, post a link to an example (especially if it fails) otherwise I won't help you (I'm not a mind reader!).

Good luck,
Mike

[gp_330]

Dear mwinter,

Thanks for the help thus far with the code, I did try using the code that you gave me, but unfortunately it didn't work. I am sorry for not putting up a version of the page up for you to look at as I can understand that this makes helping me much more difficult. I have now uploaded a version of the site up onto:

http://www.theplayground.co.za/andreoli/terms.aspx

You will see that the menu navigation is at the top of the page (I know that some of the images are missing, so please ignore them).

Once again, thanks very much, I really appreciate your help on this issue, as I am sure I wouldn't be able to figure it out without your help.

GP

[mwinter]

Sorry, I was being a little dense when I wrote that.

The problem is that the attachEvent method causes event listeners to work differently than with other registration mechanism. The this operator typically refers to the element that the listener is attached to, but with attachEvent, the operator refers to the global object. This is quite useless, and why I don't usually use it.

The changes are simple, but it's probably simpler to just repost the code. It also registers all five menus.

var initialiseMenu = function() {
    var menus = [];

    function setClass() {
        if (!/(^|\s)over(\s|$)/.test(this.className))
            this.className += ' over';
    }
    function removeClass() {
        this.className = this.className.replace(/(^|\s)over(\s|$)/, ' ');
    }
    window.attachEvent('onunload', function() {
        for (var i = 0; i < menus.length; ++i)
            menus[i].onmouseover = menus[i].onmouseout
                = null;
        menus.length = 0;
        window.detachEvent('onunload', arguments.callee);
    });

    return function(id) {
        var menu = document.getElementById(id),
            nodes = menu.childNodes;

        for (var index = 0, length = nodes.length, node; index < length; ++index) {
            node = nodes[index];
            if (node.tagName == 'LI') {
                node.onmouseover = setClass;
                node.onmouseout = removeClass;
            }
        }
        menus[menus.length] = menu;
    };
}();

window.attachEvent('onload', function() {
    initialiseMenu('menu1');
    initialiseMenu('menu2');
    initialiseMenu('menu3');
    initialiseMenu('menu4');
    initialiseMenu('menu5');

    window.detachEvent('onload', arguments.callee);
});

If you saved it as ie-menu.js, and placed it with your other scripts, you'd include it with:

<!--[if IE]><script type="text/javascript" src="Javascripts/ie-menu.js"></script><![endif]-->

Hope that helps,
Mike