views:

177

answers:

2

I am using Safalra's javascript to create a collapsible list. The script works across several browsers with no problem. However, when I apply the javascript to my own list, it fails to act as expected when I use IE (I'm using 7 at the moment). It simply writes the list, without the expand and contract images.

I copied the Safalra's javascript precisely, so I assume the error must be in my own list. This is how I generated my list:

<body onLoad="makeCollapsible(document.getElementById('libguides'));">
<ul id="libguides">
 <script type="text/javascript" src="http://api.libguides.com/api_subjects.php?iid=54&amp;amp;more=false&amp;amp;format=js&amp;amp;guides=true&amp;amp;break=li"&gt;&lt;/script&gt;
</ul>

(Yes, I do close the body tag eventually.) When I run this in IE, it tells me that line 48 is causing the problem, which appears to be:

node.onclick=createToggleFunction(node,list);

Here's the entire function:

function makeCollapsible(listElement){

  // removed list item bullets and the sapce they occupy
  listElement.style.listStyle='none';
  listElement.style.marginLeft='0';
  listElement.style.paddingLeft='0';

  // loop over all child elements of the list
  var child=listElement.firstChild;
  while (child!=null){

    // only process li elements (and not text elements)
    if (child.nodeType==1){

      // build a list of child ol and ul elements and hide them
      var list=new Array();
      var grandchild=child.firstChild;
      while (grandchild!=null){
        if (grandchild.tagName=='OL' || grandchild.tagName=='UL'){
          grandchild.style.display='none';
          list.push(grandchild);
        }
        grandchild=grandchild.nextSibling;
      }

      // add toggle buttons
      var node=document.createElement('img');

      node.setAttribute('src',CLOSED_IMAGE);
      node.setAttribute('class','collapsibleClosed');
      node.onclick=createToggleFunction(node,list);
      child.insertBefore(node,child.firstChild);

    }

I confess I'm too much of a javascript novice to understand why that particular line of code is causing the error. I looked at some of the other questions here, and was wondering if it might be a problem with setAttribute?

Thanks in advance.

Edited to add:

Here's the code for the createToggleFunction function. The whole of the script is just these two functions (plus declaring variables for the images).

function createToggleFunction(toggleElement,sublistElements){

  return function(){

    // toggle status of toggle gadget
    if (toggleElement.getAttribute('class')=='collapsibleClosed'){
      toggleElement.setAttribute('class','collapsibleOpen');
      toggleElement.setAttribute('src',OPEN_IMAGE);
    }else{
      toggleElement.setAttribute('class','collapsibleClosed');
      toggleElement.setAttribute('src',CLOSED_IMAGE);
    }

    // toggle display of sublists
    for (var i=0;i<sublistElements.length;i++){
      sublistElements[i].style.display=
          (sublistElements[i].style.display=='block')?'none':'block';
    }

  }

}

Edited to add (again):

Per David's suggestion, I changed all instances of setAttribute & getAttribute...but clearly I did something wrong. IE is breaking at the 1st line (which is simply the doctype declaration) and at line 49, which is the same line of code where it was breaking before:

node.onclick=createToggleFunction(node,list);

Here's the first function as written now:

function makeCollapsible(listElement){

  // removed list item bullets and the sapce they occupy
  listElement.style.listStyle='none';
  listElement.style.marginLeft='0';
  listElement.style.paddingLeft='0';

  // loop over all child elements of the list
  var child=listElement.firstChild;
  while (child!=null){

    // only process li elements (and not text elements)
    if (child.nodeType==1){

      // build a list of child ol and ul elements and hide them
      var list=new Array();
      var grandchild=child.firstChild;
      while (grandchild!=null){
        if (grandchild.tagName=='OL' || grandchild.tagName=='UL'){
          grandchild.style.display='none';
          list.push(grandchild);
        }
        grandchild=grandchild.nextSibling;
      }

      // add toggle buttons
      var node=document.createElement('img');

      node.src = CLOSED_IMAGE;
      node.className = 'collapsibleClosed'; 
      node.onclick=createToggleFunction(node,list);
      child.insertBefore(node,child.firstChild);

    }

    child=child.nextSibling;
  }

}

And here's the second function:

function createToggleFunction(toggleElement,sublistElements){

  return function(){

    // toggle status of toggle gadget

    // Use foo.className = 'bar'; instead of foo.setAttribute('class', 'bar');

    if (toggleElement.className == 'collapsibleClosed') {
     toggleElement.className = 'collapsibleOpen';
     toggleElement.src = OPEN_IMAGE;
    } else {
     toggleElement.className = 'collapsibleClosed';
        toggleElement.src = CLOSED_IMAGE; 
    }

    // toggle display of sublists
    for (var i=0;i<sublistElements.length;i++){
      sublistElements[i].style.display=
          (sublistElements[i].style.display=='block')?'none':'block';
    }

  }

}
A: 
node.onclick=createToggleFunction(node,list);

That is probably not what you want. Does createToggleFunction return a function? If it doesn't, then I bet you meant this:

node.onClick = function() { createToggleFunction(node, list); };

If my guess is right then the way you have it will set the onClick event handler to be the result of createToggleFunction, not a function like it needs to be.

geowa4
Looks like createToggleFunction does return a function...I'll edit my original post and add the code.
Laura
A: 

Internet Explorer (until version 8, and then only in best standards mode) has a very broken implementation of setAttribute and getAttribute.

It effectively looks something like this:

function setAttribute(attribute, value) {
    this[attribute] = value;

function getAttribute(attribute, value) {
    return this[attribute];
}

This works fine iif the attribute name matches the property name, and the property takes a string value.

This isn't the case for the class attribute, where the matching property is className.

Use foo.className = 'bar'; instead of foo.setAttribute('class', 'bar');

David Dorward
Laura