Problem making previousSibling ignore textnodes?

[shachi]

Hello everyone,

I encountered a problem with the previousSibling that it returns textnodes even when there is simply a newline character between the elements:

<span></span>\n
<span></span>

So I created this function which doesn't seem to be working:

Object.prototype.pSib = function(){
while(this.previousSibling.nodeType == 3){
var node = this.previousSibling;
}
return node;
}

but each time I run this the script goes into an infinite loop.

Any ideas how I can fix this?

Thanks for your time in this post.

[DimX]

It goes into an infinite loop, because you don't remove the text node, so it runs on the same text node all the time.

Object.prototype.pSib = function() {
    while(this.previousSibling.nodeType == 3) {
        this.parentNode.removeChild(this.previousSibling);
    }
    return node;
}

[Twey]

Ouch. Never modify the Object, man.

[shachi]

DimX: Thanks!!!

Twey: Is it bad to modify the Object? If so can you tell me why? Can I modify everything else like Function, Regexp, Math, et cetera?

[Twey]

It's OK to do so if the method truly belongs to the class, but even that may lead to unexpected results. Modifying Object, however, modifies every single Javascript object in the context, which isn't pretty.

[shachi]

if the method truly belongs to the class

I didn't get it, how could a method truly belong to a class?

[mwinter]

Object.prototype.pSib = function(){
while(this.previousSibling.nodeType == 3){
var node = this.previousSibling;
}
return node;
}

You don't actually attempt to walk the document tree. You just make the same comparison over and over again. Text nodes aren't necessarily the only non-Element node that might encounter.

If you're looking for Element nodes, look for them explicitly. Below is a set of traversal methods. The second argument to methods like getFirstChild and getPreviousSibling is a function object reference. This function is called to examine the node (the first, and only, argument to that function) and return a boolean indicating whether it's acceptable.

var Tools = function () {
        return {
            createElementCallback: function (tagName) {
                return function (node) {
                    return isElement(node, tagName);
                };
            },
            getFirstChild: function (parent, test) {
                var node = parent.firstChild;

                if (node) return test(node) ? node : this.getNextSibling(node, test);
                return null;
            },
            getLastChild: function (parent, test) {
                var node = parent.lastChild;

                if (node) return test(node) ? node : this.getPreviousSibling(node, test);
                return null;
            },
            getNextSibling: function (node, test) {
                while ((node = node.nextSibling))
                    if (test(node)) return node;
                return null;
            },
            getPreviousSibling: function (node, test) {
                while ((node = node.previousSibling))
                    if (test(node)) return node;
                return null;
            },
            isElement: isElement
        };

        function isElement(node, tagName) {
            return (node.nodeType == 1)
                && (tagName ? (node.nodeName == tagName) : (node.nodeName != '!'));
        }
    }();

For example,

Tools.getFirstChild(parent, function (node) {
    return (Tools.isElement(node) &&
        ((node.nodeName == 'UL') || (node.nodeName == 'OL')));
});

will return the first list (ordered or unordered) element, skipping any other nodes (including uninteresting elements). If there is no such element, the return value will be null.

The createElementCallback method simplifies looking for a particular "type" of element. For example,

var isAnchor = Tools.createElementCallback('A');

Tools.getNextSibling(element, isAnchor);

will return the first anchor (a) element that follows element, or null.

Note that the tag names are case-sensitive (and will be upper-case in HTML/pseudo-XHTML).

It goes into an infinite loop, because you don't remove the text node, so it runs on the same text node all the time.

Removing the text node is a possible approach, but only if removing the text node is sensible. It might not always be, so it cannot be a general solution.

Ouch. Never modify the Object, man.

Never? If it solves a particular problem properly, then do it. However, it's surely inappropriate, here.

I didn't get it, how could a method truly belong to a class?

If a method can apply to all instances of a "class" (there are no classes as such in ECMAScript) then one can consider it to "belong". However, it doesn't, here: a RegExp object doesn't have a nextSibling property, does it?

Are you trying to modify the prototype object of DOM nodes? That's not going to work in all browsers. Not all of them even have prototype objects for host objects.

Mike

[shachi]

mwinter: Thanks for the function and definitions but how do I use the object you provided to iterate through the dom ignoring text-nodes? For e.g if there are 10 buttons on a page and each has an action that provides it's id to a function which tells the user the button's previous and next sibling(which was supposed to be the button before the clicked one) but unfortunately it returns a textnode. How do I fix this?

[shachi]

DimX: your script works great with some fixes but isn't there any other way in which I should not destroy the text-node?

mwinter: I am looking forward for your reply.

[mwinter]

Thanks for the function and definitions but how do I use the object you provided to iterate through the dom ignoring text-nodes? For e.g if there are 10 buttons on a page and each has an action that provides it's id to a function which tells the user the button's previous and next sibling(which was supposed to be the button before the clicked one) but unfortunately it returns a textnode. How do I fix this?

Based on that description, skipping text nodes isn't so much the issue as finding the right sort of element. So, the first step is creating a callback function that will identify those elements. I'll assume for the moment that simply checking the node is an input element will do; this can be accomplished using the createElementCallback method:

var isButton = Tools.createElementCallback('INPUT');

The function object assigned to the isButton variable is essentially:

function isButton(node) {
    return Tools.isElement(node, 'INPUT');
}

If you wanted to ensure that the element was actually a button rather than any other type of input element, the latter function could be modified to check the type property, for example.

Once the callback has been written, it's then just a matter of calling the getPreviousSibling and getNextSibling methods, passing a reference to the activated button as the first argument, and a reference to the callback as the second:

function findNeighbours(element) {
    var isButton = Tools.createElementCallback('INPUT'),
        nextSibling = Tools.getNextSibling(element, isButton),
        previousSibling = Tools.getPreviousSibling(element, isButton);

    /* ... */
}

If nextSibling is null, no buttons follow the one passed to the function. Likewise, if previousSibling is null, no buttons precede element.

If all you cared about was finding the next element, no matter what sort of element it was, then the existing isElement method will do:

function findNeighbours(element) {
    var nextSibling = Tools.getNextSibling(element, Tools.isElement),
        previousSibling = Tools.getPreviousSibling(element, Tools.isElement);

    /* ... */
}

Hope that helps,
Mike