fix(forEach): differentiate objects with a length property and array …

…like objects fixes angular#1840 when an object has a numeric length property: if an object has a length property with a number value of 0, it will be considered like an array like object if the value is positive and the object looks like a full dense array with a property named as length minus one, it will be considered like an array like object Signed-off-by: Gonzalo Ruiz de Villa <[email protected]>
gonzaloruizdevilla · Feb 9, 2013 · f24aa5c · f24aa5c
1 parent 649b892
commit f24aa5c
Show file tree

Hide file tree

Showing 2 changed files with 76 additions and 4 deletions.
diff --git a/src/Angular.js b/src/Angular.js
@@ -101,13 +101,15 @@ function forEach(obj, iterator, context) {
       }
     } else if (obj.forEach && obj.forEach !== forEach) {
       obj.forEach(iterator, context);
-    } else if (isObject(obj) && isNumber(obj.length)) {
+    } else if (isArrayLike(obj)) {
       for (key = 0; key < obj.length; key++)
         iterator.call(context, obj[key], key);
     } else {
-      for (key in obj) {
-        if (obj.hasOwnProperty(key)) {
-          iterator.call(context, obj[key], key);
+      if(obj.hasOwnProperty) {
+        for (key in obj) {
+          if (obj.hasOwnProperty(key)) {
+            iterator.call(context, obj[key], key);
+          }
         }
       }
     }
@@ -348,6 +350,18 @@ function isArray(value) {
   return toString.apply(value) == '[object Array]';
 }
 
+function isArrayLike(obj) {
+  var length = obj.length;
+  if (isWindow(obj)) {
+    return false;
+  }
+  return isArray(obj) ||
+    (
+      isObject(obj) && 
+      isNumber(length) && 
+      (length === 0 || length > 0 && (length - 1) in obj)
+    );
+}
 
 /**
  * @ngdoc function

diff --git a/test/AngularSpec.js b/test/AngularSpec.js
@@ -267,6 +267,64 @@ describe('angular', function() {
 
       expect(log).toEqual(['bar:barVal', 'baz:bazVal']);
     });
+
+    it('should iterate identify and iterate array like objects', function() {
+      var log;
+
+      log = [];
+      forEach(
+          [1,2,3], 
+          function(value, key) { log.push(key + ':' + value) }
+      );
+      expect(log).toEqual(['0:1', '1:2', '2:3']);
+
+      log = [];
+      forEach(
+          {
+            'bar' : 'bar',
+            'length': 2
+          }, 
+          function(value, key) { log.push(key + ':' + value) }
+      );
+      expect(log).toEqual(['bar:bar', 'length:2']);
+
+      log = [];
+      forEach(
+          jqLite("<p><span>s1</span><span>s2</span></p>").find("span"), 
+          function(value, key) { log.push(key + ':' + value.innerHTML) }
+      );
+      expect(log).toEqual(['0:s1', '1:s2']);
+
+      log = [];
+      forEach(
+          jqLite("<p><span>s1</span><span>s2</span></p>").find("b"), 
+          function(value, key) { log.push(key + ':' + value.innerHTML); }
+      );
+      expect(log.length).toBe(0);
+
+      log = [];
+      forEach(
+          document.getElementsByTagName("x"), 
+          function(value, key) { log.push(true) }
+      );
+      expect(log.length).toBe(0);
+
+      log = [];
+      forEach(
+          window, 
+          function(value, key) { log.push(true) }
+      );
+      expect(log.length).toBeGreaterThan(0);
+
+      //a plain old object with length property with 0 value is treated like an array
+      log = [];
+      forEach(
+          {"prop1":"value1","length":0}, 
+          function(value, key) { log.push(true) }
+      );
+      expect(log.length).toBe(0);
+
+    });
   });