#30 ✓resolved
larry.karnowski (at gmail)

Prototype $$ method fails due to missing className on DOMNode

Reported by larry.karnowski (at gmail) | February 1st, 2009 @ 06:19 AM | in 1.0 Release

Prototype's $$("class") method, which is akin to jQuery's $(".class") method, is failing because it's traversing the DOM and expecting each node to have a defined, non-null class name value. However, DOMNode has no "className" value.

Comments and changes to this ticket

  • larry.karnowski (at gmail)

    larry.karnowski (at gmail) February 1st, 2009 @ 06:27 AM

    I've added a Prototype compatibility test and a patch for this on my GitHub fork. Here are the commits:

    Prototype compat test: http://github.com/Lawjoskar/env-...

    Patch: http://github.com/Lawjoskar/env-...

  • Thatcher

    Thatcher February 16th, 2009 @ 08:58 AM

    • Milestone set to 1.0 Release
    • State changed from “new” to “open”
    • Assigned user set to “Thatcher”

    Thanks Larry.

    I'm confused about why the correct place is on the DOMNode, since most dom nodes in firebug don't have this property. Try examining the childNodes property of an average element you'll see that text nodes and attributes dont have this, just elements.

    It is a property of the HTMLElement but maybe some element is being searched that was not created as a dom element and not an html element.

    Let's put the patch there first and see if that corrects the issue. Can you explain the error more specifically.

    PS I might borrow your prototype compat test to get myself started here.

    Thatcher

  • Thatcher

    Thatcher February 16th, 2009 @ 09:31 AM

    Ok I think the relavant code from prototype is

    
    var nodes = $(element).getElementsByTagName('*');
        className = ' ' + className + ' ';
    
        for (var i = 0, child, cn; child = nodes[i]; i++) {
          if (child.className && (cn = ' ' + child.className + ' ') && (cn.include(className) ||
              (classNames && classNames.all(function(name) {
                return !name.toString().blank() && cn.include(' ' + name + ' ');
              }))))
            elements.push(Element.extend(child));
        }
        return elements;
      };
    

    so it is iterating what should be a list of html elements so having className on the html Element interface should be enough. I need to double check but I think the actual problem may be in the getElementByTagName code. for the case where the name is '*'.

  • Thatcher

    Thatcher February 16th, 2009 @ 09:42 AM

    Yeah so the cleaner patch in this case is to getElementsByTagNameRecursive

    Before we had:

    
    if (elem.nodeType == DOMNode.ELEMENT_NODE || elem.nodeType == DOMNode.DOCUMENT_NODE) {
        
            if(elem.nodeName.toUpperCase() == tagname.toUpperCase() || (tagname == "*")){
                __appendChild__(nodeList, elem);               // add matching node to nodeList
            }
    

    This put the Document Node in the list. ;(

    So I changed it to this:

    
    if (elem.nodeType == DOMNode.ELEMENT_NODE || elem.nodeType == DOMNode.DOCUMENT_NODE) {
        
            if(elem.nodeType !== DOMNode.DOCUMENT_NODE && 
                ((elem.nodeName.toUpperCase() == tagname.toUpperCase()) || 
                    (tagname == "*")) ){
                __appendChild__(nodeList, elem);               // add matching node to nodeList
            }
    

    so it'll only add elements to the list but still allow recursion on the docuement node.

    You test passes then!

    I'll close this ticket when I commit.

  • Thatcher

    Thatcher February 17th, 2009 @ 04:33 AM

    • State changed from “open” to “resolved”

Please Sign in or create a free account to add a new ticket.

With your very own profile, you can contribute to projects, track your activity, watch tickets, receive and update tickets through your email and much more.

New-ticket Create new ticket

Create your profile

Help contribute to this project by taking a few moments to create your personal profile. Create your profile ยป

a javascript browser environment

People watching this ticket

Tags

Referenced by

Pages