PDA

View Full Version : [OPEN] [FIXED][3.0 CORE svn 377] Bug in private isIterable() method



mystix
24 Jun 2009, 6:16 PM
the private isIterable() method contains an incorrect check for NodeList#item() and NodeList#length (it checks for these on a string), and an unnecessary strict String comparison.

the following changes in red resolve these issues.


isIterable = function(v){
//check for array or arguments
if(Ext.isArray(v) || v.callee){
return true;
}
var t = toString.call(v);
//check for node list type
if(/NodeList|HTMLCollection/.test(t)){
return true;
}

//NodeList has an item and length property
return (Ext.isFunction(v.item) && Ext.isNumber(v.length));
},

but brings up a separate issue:- if v is indeed a NodeList, the final portion of code is never reached. :-?


additionally, the new Ext.urlAppend() and Ext.isXXX() methods also perform an unnecessary strict number/string checks.
e.g.


url.indexOf('?') === -1 // indexOf() always returns an integer

toString.apply(v) === '[object Function]' // perhaps toString() should be redefined as toString = Object.prototype.toString.apply to reduce code

typeof v === 'number' // typeof() always returns a string


and the new Ext.isDefined() method could be simplified to just


isDefined: function(v){
return v !== undefined;
}

evant
24 Jun 2009, 7:24 PM
I've made those few fixes in SVN, I'm not sure what you mean by



but brings up a separate issue:- if v is indeed a NodeList, the final portion of code is never reached.


We're assuming if the type is a NodeList or HTMLCollection we don't need to go any further.

FYI JSLint complains at times if you don't use strict comparisons with integers. I don't think it cares about strings though.

mystix
24 Jun 2009, 7:34 PM
thanks for the JSLint bit -- never knew it was that picky :(


as for the NodeList bit:
if v is indeed a NodeList, then isIterable() exits at the following point:


//check for node list type
if(/NodeList|HTMLCollection/.test(t)){
return true;
}


which means that the subsequent NodeList#item() and NodeList#length check is never reached:


// NodeList has an item and length property
return (Ext.isFunction(v.item) && Ext.isNumber(v.length));


let me know if that still doesn't clear things up.

evant
24 Jun 2009, 7:36 PM
Yep, that's the intent, if the type returns NodeList or HTMLCollection there's no need to make the subsequent checks. IIRC It's only IE that doesn't return the type as either NodeList or HTMLCollection.

mystix
24 Jun 2009, 7:39 PM
ah IE... makes sense then.

guess you can wrap up this report. thanks!

mystix
24 Jun 2009, 7:52 PM
weird... IE5, FF1, and Opera 9 support NodeLists:
http://www.w3schools.com/dom/dom_nodelist.asp

anyhoos... if anyone finds any browser which doesn't support either, just chime back here.
it'll be good to know which browsers croak on either.

evant
24 Jun 2009, 8:26 PM
I used:



var x = document.getElementsByTagName('body');
alert(typeof x);


In IE I just got [object Object].

mystix
24 Jun 2009, 8:30 PM
typeof()...

now i get it. thanks :)