From 205eb6530a60e3bbb959dbcc410aeff50371ed6e Mon Sep 17 00:00:00 2001 From: Pete Bacon Darwin Date: Tue, 18 Dec 2012 20:42:25 +0000 Subject: [PATCH] fix(jqLite): children() should only return elements The jQuery implementation of children only returns child nodes of the given element that are elements themselves. The previous jqLite implementation was returning all nodes except those that are text nodes. Use jQLite.contents() to get all the child nodes. The jQuery implementation of contents returns [] if the object has no child nodes. The previous jqLite implementation was returning undefined, causing a stack overflow in test/testabilityPatch.js when it tried to `cleanup()` a window object. The testabilityPatch was incorrectly using children() rather than contents() inside cleanup() to iterate down through all the child nodes of the element to clean up. --- src/jqLite.js | 4 ++-- test/jqLiteSpec.js | 11 ++++++----- test/testabilityPatch.js | 2 +- 3 files changed, 9 insertions(+), 8 deletions(-) diff --git a/src/jqLite.js b/src/jqLite.js index 8305f0c30f2c..a4e86612694d 100644 --- a/src/jqLite.js +++ b/src/jqLite.js @@ -647,14 +647,14 @@ forEach({ children: function(element) { var children = []; forEach(element.childNodes, function(element){ - if (element.nodeName != '#text') + if (element.nodeType === 1) children.push(element); }); return children; }, contents: function(element) { - return element.childNodes; + return element.childNodes || []; }, append: function(element, node) { diff --git a/test/jqLiteSpec.js b/test/jqLiteSpec.js index 01f9b9ae8138..5d65d8ccbe8d 100644 --- a/test/jqLiteSpec.js +++ b/test/jqLiteSpec.js @@ -925,8 +925,8 @@ describe('jqLite', function() { describe('children', function() { - it('should select non-text children', function() { - var root = jqLite('
').html('before-
after-'); + it('should only select element nodes', function() { + var root = jqLite('
before-
after-'); var div = root.find('div'); var span = root.find('span'); expect(root.children()).toJqEqual([div, span]); @@ -936,10 +936,11 @@ describe('jqLite', function() { describe('contents', function() { it('should select all children nodes', function() { - var root = jqLite('
').html('before-
after-'); + var root = jqLite('
').html('before-
after-'); var contents = root.contents(); - expect(contents.length).toEqual(4); - expect(jqLite(contents[0]).text()).toEqual('before-'); + expect(contents.length).toEqual(5); + expect(contents[0].textContent).toEqual(' some comment '); + expect(contents[1].textContent).toEqual('before-'); }); }); diff --git a/test/testabilityPatch.js b/test/testabilityPatch.js index ac22c72c05cd..d55ff0159ca3 100644 --- a/test/testabilityPatch.js +++ b/test/testabilityPatch.js @@ -78,7 +78,7 @@ function dealoc(obj) { function cleanup(element) { element.unbind().removeData(); - for ( var i = 0, children = element.children() || []; i < children.length; i++) { + for ( var i = 0, children = element.contents() || []; i < children.length; i++) { cleanup(jqLite(children[i])); } }