Opened 10 years ago

Closed 10 years ago

Last modified 10 years ago

#10026 closed defect (wontfix)

dojo.NodeList.attr() return value for nonexistent attributes

Reported by: Les Owned by: anonymous
Priority: high Milestone: tbd
Component: General Version: 1.3.2
Keywords: dojo.NodeList-traverse Cc:
Blocked By: Blocking:

Description

There's a minor inconsistency in the way dojo.NodeList?.attr() returns value for nonexistent attributes.

Go here:

http://archive.dojotoolkit.org/nightly/dojotoolkit/dojo/tests/NodeList-manipulate.html

Try:

dojo.require('dojo.NodeList-traverse');
dojo.query('form>*').attr('id'); // result: [null, "inputTextArea", null]

dojo.require('dojo.NodeList-traverse');
dojo.query('form>*').attr('class'); // result: ["myClass", "", "myClass"]

jQuery('form>*').attr('id'); // ""

I was expecting that NodeList?.attr() would return an empty string for nonexistent attributes.

I don't mind that dojo.query().attr() beahavior doesn't match jQuery().attr(). IMO dojo behavior is what should be expected. My comment is about the return value.

Change History (12)

comment:1 Changed 10 years ago by Les

Also, try the following code on the same page. You will see that first() and last() return different results, although both are null.

dojo.query('form *').attr('id'); // returns array, see below

[null, "inputText", "inputRadio1", "inputRadio2", "inputCheckBox1", "inputCheckBox2", "inputTextArea", null, "inputSelect1", null, null, null, "inputSelect2", null, null, null]
dojo.query('form *').attr('id').first(); // returns []
dojo.query('form *').attr('id').last(); // returns [null]

comment:2 Changed 10 years ago by Les

Try this on the same page. You will see that at() removes null ids, which is not correct. It should return either return null or "".

dojo.require('dojo.NodeList-traverse');

dojo.query('form *').attr('id').at(0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15);

Result:

["inputText", "inputRadio1", "inputRadio2", "inputCheckBox1", "inputCheckBox2", "inputTextArea", "inputSelect1", "inputSelect2"]

comment:3 Changed 10 years ago by Les

Try:

dojo.require('dojo.NodeList-traverse');

dojo.query('form *').attr('class').at(0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15);

The result is not what I'd expect.

["myClass", "myClass"]

comment:4 Changed 10 years ago by James Burke

Resolution: wontfix
Status: newclosed

dojo.NodeList?.attr() returns a plain array, not a decorated array via NodeList?. I am surprised the first, last, and at tests work -- the first() test I tried for me in Firefox 3.5 resulted in an error.

I believe the return value is consistent: it could be that an attribute is set to an empty string, vs not existing at all. And I was able to do this with the first element that had no ID, I set the id="" and attr gave back "".

So I think this is working as designed.

comment:5 Changed 10 years ago by Les

Resolution: wontfix
Status: closedreopened

dojo.NodeList??.attr() returns a plain array, not a decorated array via NodeList??.

Try: console.dir(dojo.query('form *').attr('id'))

I see methods such as _stash() and _NodeListCtor() This is not a plain array. I'm not sure why there's no first() or last(). I think this is something to check, so I will reopen.

comment:6 Changed 10 years ago by Les

Also, why this returns a compressed NodeList?? See above.

dojo.require('dojo.NodeList-traverse');

dojo.query('form *').attr('class').at(0,1,2,3,4,5,6,7,8,9,10,11,12,13,14,15);

comment:7 Changed 10 years ago by Les

dojo.require('dojo.NodeList?-traverse') adds first() and last()

comment:8 Changed 10 years ago by Les

The return value for attr() is not consistent with the inline doc. I'm not passing value, but it returns NodeList?.

//	returns:
//		if no value is passed, the result is an array of attribute values
//		If a value is passed, the return is this NodeList
return; // dojo.NodeList
return; // Array

Also, the inline doc for position() is not consistent withe the return value. It returns a NodeList?, but this is not what the doc says.

//		Returns border-box objects (x/y/w/h) of all elements in a node list
//		as an Array (*not* a NodeList).

comment:9 Changed 10 years ago by bill

About the return value, NodeList *is* an array, so attr() is working according to spec. This looks like #10009 over again.

To put it another way, getChildren() is documented as returning a dijit._Widget[], but getChildren() on a Menu will return dijit.MenuItem[] rather than a plain dijit._Widget[]. That's not a bug.

The problems with at() and first()/last() are separate and should be filed as separate tickets.

As for attr('id') and attr('class') working differently (which was the original point of this ticket), I wonder if that can be explained because all nodes by default have a class attribute. It seems like this is really about how dojo.attr() works, in which case the ticket should be filed against that component.

comment:10 in reply to:  9 Changed 10 years ago by Les

Replying to bill:

About the return value, NodeList *is* an array, so attr() is working according to spec.

The inline doc for attr() distinguished between a NodeList? and an Array being returned:

"if no value is passed, the result is an array of attribute values. If a value is passed, the return is this NodeList"

My point is that actually there's no distinction, so attr() is not working according to spec. One could reason that NodeList? is an Array, so the return value is as spec'd, but the doc clearly makes a distinction.

comment:11 Changed 10 years ago by James Burke

Resolution: wontfix
Status: reopenedclosed

I believe the behavior of the attr() return array being decorated with NodeList? functions occurs from the changes elazutkin did for dojo.NodeList? to consolidate functionality that converts a regular dojo method into a dojo.NodeList? method that operates on a list of items.

Specifically, dojo.NodeList?'s attr is defined via adaptWithCondition(), which uses this.map(), which then uses this._wrap() to wrap the returned array with the NodeList? methods.

So while it does technically change the return type for getting attrs from a plain array to a decorated array, I think this is OK. For the future of the Core methods, I am leaning towards wrapping more basic things in our functions, to add more syntatic sugar (but only if you call a dojo-specific method first).

I still like it though that the documents say it is a normal array, so folks are more inclined to use it as such.

So I agree it is a change over what is documented, but the documentation is more strict than the actual code, and I prefer not to try to muddy either the docs to explain the more nuanced situation, or increase the size of dojo.NodeList? to strictly return a plain array.

Interesting find though. I was not aware of this effect until now. Neat.

comment:12 Changed 10 years ago by bill

The inline doc for attr() distinguished between a NodeList and an Array being returned: "if no value is passed, the result is an array of attribute values. If a value is passed, the return is this NodeList"

BTW, the key word is this; it's pointing out that the original list of nodes is returned, for chaining purposes.

Note: See TracTickets for help on using tickets.