Comments Considered Harmful?

Commenting code. Normally a good thing, right (the views of those crazy XP people notwithstanding)?

You wouldn’t have thought that adding comments could actually stop your code working, could you? Yet I’ve been writing XBL recently, and hit not one but two situations where exactly that happened.

In XBL, you write a binding which represents a widget, and you can then write methods to call on any element which you’ve bound that binding to. So you can do something like myWidget.throb(). If you want to use your method to fiddle with the anonymous content of your widget, the usual pattern used to get hold of it is:

var rootNode = document.getAnonymousNodes(this)[0];

However, if you add a comment at the start of the <content>, node 0 is now a comment node, and all your methods stop working.

Secondly, and perhaps more seriously, I’ve just debugged a crash I started getting, and it turns out that some of the code in nsXBLPrototypeBinding.cpp, which is invoked when you change an XBL attribute via the DOM, doesn’t cope very well with comment nodes (it doesn’t check a return value for null). Removing all my comments prevents the crash. In other words, Mozilla crashed because I had commented my code too well…

5 thoughts on “Comments Considered Harmful?

  1. Well, I can recommend to you using ‘getAnonymousElementByAttribute’.
    Something like this:
    document.getAnonymousElementByAttribute(document.getBindingParent(this),’id’,’uniqueid’);

    document.getBindingParent(this) you can use in case you have an onclick event handler on an anonymous element. That way you can get the binding parent.

    I prefer not to use document.getAnonymousNodes anymore, because it is very easy to break this function just by reordering your xbl content.

  2. I’ve found an IE bug where the presence of comments in HTML code causes CSS to stop working correctly. Unfortunately I can’t remember how to reproduce it but it was a very simple example with a couple of nested divs, a couple of comments, and a couple of stylesheet rules. Removing the comments made it render correctly.

    That wasn’t “by design” like this is, though. I always thought there was something wrong with the getAnonymousNodes(this)[0] pattern; glad to see my gut feeling of discomfort had a reason behind it.

  3. If for some reason you can’t know the attributes of the element in question before hand, you could just iterate over the getAnonymousNodes(this) array until you hit a non-comment node. But I’ve always used the method Martijn suggested.

  4. DOM 2 TreeWalkers are your best friend.

    I created a little getChildElements() function that uses a DOM 2 TreeWalker:

    const acceptNodeFilter = {
    acceptNode: function(aNode) {
    return NodeFilter.FILTER_ACCEPT;
    },

    toString: function() {
    return “[object NodeFilter]”;
    }
    };

    const foo = {
    getChildElements: function(aNode) {
    var walker = aNode.ownerDocument.createTreeWalker(aNode, NodeFilter.SHOW_ELEMENT, acceptNodeFilter, true);
    var rv = [];
    var firstChildNode = walker.firstChild();
    if (firstChildNode) {
    rv.push(firstChildNode);
    while (walker.nextSibling()) {
    rv.push(walker.currentNode);
    }
    }
    return rv;
    }
    }

    I use this arrangement (or something nearly identical to it) in Abacus quite a bit…

  5. (Hi Gerv!)

    Comments stopping your code working?

    Some RedHat RPMs I built wouldn’t rebuild on someone else’s platform. Here’s why.

    Great…