fix(Angular.js): fix isArrayLike for unusual cases

Closes angular#10186 Closes angular#8000 Closes angular#4855 Closes angular#4751 Closes angular#10272
petebacondarwin · Oct 25, 2015 · d95b7d6 · d95b7d6
1 parent a040f02
commit d95b7d6
Show file tree

Hide file tree

Showing 2 changed files with 40 additions and 42 deletions.
diff --git a/src/Angular.js b/src/Angular.js
@@ -191,57 +191,31 @@ var
 msie = document.documentMode;
 
 
+function isNodeList(obj) {
+  return typeof obj.length == 'number' &&
+         typeof obj.item == 'function';
+}
+
 /**
  * @private
  * @param {*} obj
  * @return {boolean} Returns true if `obj` is an array or array-like object (NodeList, Arguments,
  *                   String ...)
  */
 function isArrayLike(obj) {
-  // snake case is to avoid shadowing camel-cased globals
-  var length, objExists, isNodeList, isArguments, isSomeOtherObj, is_array, is_string, is_object;
-  objExists = isDefined(obj) && obj !== null;
+
+  // `null`, `undefined` and `window` are not array-like
+  if (obj == null || isWindow(obj)) return false;
+
+  // arrays and strings are array like
+  if(isArray(obj) || isString(obj)) return true;
+
   // Support: iOS 8.2 (not reproducible in simulator)
   // "length" in obj used to prevent JIT error (gh-11508)
-  length = objExists ? "length" in Object(obj) && obj.length : false;
-  is_array = isArray(obj);
-  is_string = isString(obj);
-  is_object = isObject(obj);
-  isNodeList = objExists && obj.nodeType === 1 && length;
-	isArguments = objExists &&
-		(Object.prototype.toString.call(obj) === '[object Arguments]' ||
-		(Object.prototype.hasOwnProperty.call(obj, 'length')         &&
-		Object.prototype.hasOwnProperty.call(obj, 'callee')));
-
-  // this only works if it doesn't return 'object' from typeof and isn't another arrayLike
-  isSomeOtherObj = objExists &&
-    !isNodeList              &&
-    !is_array                &&
-    !is_string               &&
-		!isArguments             &&
-    (
-      (!is_object   &&
-      length === 0) ||
-        (
-          isNumber(length) &&
-            length >= 0    &&
-            (length - 1) in obj
-        )
-    );
+  var length = "length" in Object(obj) && obj.length;
 
-  return (
-    objExists      &&
-    !isWindow(obj) &&
-    (
-      (
-        isNodeList  ||
-        is_string   ||
-        is_array    ||
-				isArguments
-      ) ||
-      isSomeOtherObj
-    )
-  );
+  // node lists and objects with suitable length characteristics are array-like
+  return (isNumber(length) && length >= 0 && (length - 1) in obj) || isNodeList(obj);
 }
 
 /**
@@ -501,7 +475,7 @@ identity.$inject = [];
 function valueFn(value) {return function() {return value;};}
 
 function hasCustomToString(obj) {
-  return isFunction(obj.toString) && obj.toString !== Object.prototype.toString;
+  return isFunction(obj.toString) && obj.toString !== toString;
 }
 
 

diff --git a/test/AngularSpec.js b/test/AngularSpec.js
@@ -1055,15 +1055,39 @@ describe('angular', function() {
   });
 
   describe('isArrayLike', function() {
+
     it('should return false if passed a number', function() {
       expect(isArrayLike(10)).toBe(false);
     });
+
     it('should return true if passed an array', function() {
       expect(isArrayLike([1,2,3,4])).toBe(true);
     });
+
     it('should return true if passed an object', function() {
       expect(isArrayLike({0:"test", 1:"bob", 2:"tree", length:3})).toBe(true);
     });
+
+    it('should return true if passed arguments object', function() {
+      function test(a,b,c) {
+        expect(isArrayLike(arguments)).toBe(true);
+      }
+      test(1,2,3);
+    });
+
+    it('should return true if passed a nodelist', function() {
+      var nodes = document.body.childNodes;
+      expect(isArrayLike(nodes)).toBe(true);
+    });
+
+    it('should return false for objects with `length` but no matching indexable items', function() {
+      var obj = {
+        a: 'a',
+        b:'b',
+        length: 10
+      };
+      expect(isArrayLike(obj)).toBe(false);
+    });
   });