This is a demo of the script explained in Accessible expanding and collapsing menu.

The HTML

The HTML that appeared in the article is this (shortened, and with id values translated from Swedish):

<div id="menu">
<div id="mainmenu"><a href="#" onclick="toggle('submenu1')">Category 1</a></div>
<div id="submenu1" style="display:none">
<div id="submenu"><a href="#">Submenu 1</a></div>
<div id="submenu"><a href="#">Submenu 1</a></div>
</div>
<div id="mainmenu"><a href="#" onclick="toggle('submenu2')">Category 2</a></div>
<div id="submenu2" style="display:none">
<div id="submenu"><a href="#">Submenu 2</a></div>
<div id="submenu"><a href="#">Submenu 2</a></div>
</div>
<div id="mainmenu"><a href="#" onclick="toggle('submenu3')">Category 3</a></div>
<div id="submenu3" style="display:none">
<div id="submenu"><a href="#">Submenu 3</a></div>
<div id="submenu"><a href="#">Submenu 3</a></div>
</div>
</div>

This markup is problematic for several reasons:

1. id values must be unique, and may not be shared by several elements on the same page.
2. Using CSS to hide something (via style="display:none") and JavaScript to show it makes the hidden content inaccessible to people who browse with CSS on and JavaScript off.
3. It uses inline CSS.
4. It uses inline event handlers.
5. The menu consists of several lists of links, so list elements should be used instead of div elements.

I replaced the HTML with this:

<ul class="menu">
<li><a href="#">Category 1</a>
<ul>
<li><a href="#">Submenu 1a</a></li>
<li><a href="#">Submenu 1b</a></li>
</ul>
</li>
<li><a href="#">Category 2</a>
<ul>
<li><a href="#">Submenu 2a</a></li>
<li><a href="#">Submenu 2b</a></li>
</ul>
</li>
<li><a href="#">Category 3</a>
<ul>
<li><a href="#">Submenu 3a</a></li>
<li><a href="#">Submenu 3b</a></li>
</ul>
</li>
 </ul>

The menu now consists of nested lists of links, the inline CSS and event handlers are gone, and a class name is used to allow for more than one menu on a page, should there be a need for that. Should the browser lack support for JavaScript, all sub menus will now be displayed, and the user is not prevented from navigating the site. Much better in my opinion.

The CSS

Next a look at the CSS used in the article (id selectors translated from Swedish):

 #menu {
font-family: Verdana, Arial, Helvetica, sans-serif;
font-size: 10px;
font-weight: normal;
left: 20px;
top: 20px;
 }
#menu #mainmenu a {
 color: #FFFFFF;
text-decoration: none;
display: block;
 background-color: #990066;
 padding-top: 2px;
 padding-right: 5px;
padding-bottom: 2px;
padding-left: 5px;
width: 150px;
position:relative;
}
#menu #mainmenu a:hover {
background-color: #CC6699;
}
#menu #submenu a {
color: #FFFFFF;
text-decoration: none;
display: block;
background-color: #CC0066;
padding-top: 2px;
padding-right: 5px;
padding-bottom: 2px;
padding-left: 5px;
width: 135px;
left: 15px;
position:relative;
}
#menu #submenu a:hover {
background-color: #CC6699;
 }

While that CSS is not invalid, it is definitely not optimal, for the following reasons:

1. It suffers from what I call selectoritis, that is the use of overly specific selectors (#menu #mainmenu etc.).
2. No use of shorthand, making the CSS bulkier than necessary.
3. It uses positioning for no apparent reason.

Here is what I replaced it with:

 .menu,
 .menu ul {
  margin:0;
  padding:0;
  list-style:none;
 }
 .menu {width:200px;}
 .menu li {
  display:block;
  margin:0;
  padding:0;
  margin-bottom:1px;
 }
 .menu a {
  display:block;
  padding:2px 5px;
  color:#000;
  background:#b0c23d;
  text-decoration:none;
 }
 .menu a:hover,
 .menu a:focus,
 .menu a:active {background:#d9dcb0;}
 .menu ul li {padding-left:15px;}
 .menu ul a {background:#ced174;}

I removed the font declarations since font-family and font-size are normally specified elsewhere in the CSS. While I was at it I also changed the colours to make looking at the menu a bit easier on my eyes, and added some padding and a bottom margin to the links.

The JavaScript

Finally a look at the JavaScript. The article uses the following (variable names translated from Swedish):

function toggle(submenu) {
if (document.getElementById(submenu).style.display == "none")
document.getElementById(submenu).style.display = "block";
else if (document.getElementById(submenu).style.display == "block")
 document.getElementById(submenu).style.display = "none"
else alert("An error occured in the menu. Contact the web master.");
 }

The problems with this script are:

1. It doesn’t check if the browser supports the method it uses.
2. It works by directly changing the element style.
3. If anything should go wrong it displays a completely useless message.

Ok, I’ll admit that the script does have an advantage over my replacement: it is a lot shorter than this:

var toggleMenu = {
 init : function(sContainerClass, sHiddenClass) {
  if (!document.getElementById || !document.createTextNode) {return;} // Check for DOM support
  var arrMenus = this.getElementsByClassName(document, 'ul', sContainerClass);
  var arrSubMenus, oSubMenu, oLink;
  for (var i = 0; i < arrMenus.length; i++) {
   arrSubMenus = arrMenus[i].getElementsByTagName('ul');
   for (var j = 0; j < arrSubMenus.length; j++) {
    oSubMenu = arrSubMenus[j];
    oLink = oSubMenu[removed].getElementsByTagName('a')[0];
    oLink.onclick = function(){toggleMenu.toggle(this[removed].getElementsByTagName('ul')[0], sHiddenClass); return false;}
    this.toggle(oSubMenu, sHiddenClass);
   }
  }
 },
 toggle : function(el, sHiddenClass) {
  var oRegExp = new RegExp("(^|\\s)" + sHiddenClass + "(\\s|$)");
  el.className = (oRegExp.test(el.className)) ? el.className.replace(oRegExp, '') : el.className + ' ' + sHiddenClass; // Add or remove the class name that hides the element
 },
/* addEvent function from http://www.quirksmode.org/blog/archives/2005/10/_and_the_winner_1.html */
 addEvent : function(obj, type, fn) {
  if (obj.addEventListener)
   obj.addEventListener(type, fn, false);
  else if (obj.attachEvent) {
   obj["e"+type+fn] = fn;
   obj[type+fn] = function() {obj["e"+type+fn](window.event);}
   obj.attachEvent("on"+type, obj[type+fn]);
  }
 },
/*
Written by Jonathan Snook, http://www.snook.ca/jonathan
Add-ons by Robert Nyman, http://www.robertnyman.com
*/
 getElementsByClassName : function(oElm, strTagName, strClassName){
     var arrElements = (strTagName == "*" && document.all)? document.all : oElm.getElementsByTagName(strTagName);
     var arrReturnElements = new Array();
     strClassName = strClassName.replace(/\-/g, "\\-");
     var oRegExp = new RegExp("(^|\\s)" + strClassName + "(\\s|$)");
     var oElement;
     for(var i=0; i<arrElements.length; i++){
         oElement = arrElements[i];      
         if(oRegExp.test(oElement.className)){
             arrReturnElements.push(oElement);
         }   
     }
     return (arrReturnElements)
 }
};
toggleMenu.addEvent(window, 'load', function(){toggleMenu.init('menu','hidden');});

The replacement script works by adding or removing a class name instead of directly changing the style property of the sub menu elements.

The addEvent() and getElementsByClassName() functions are included in the full togglemenu.js script. If you use a JavaScript library that contains similar functions (they all do, probably) I suggest using those functions instead to avoid code bloat.