Skip to content
This repository has been archived by the owner on Apr 12, 2024. It is now read-only.

Commit

Permalink
fix(jqLite): children() should only return elements
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
petebacondarwin committed Jan 9, 2013
1 parent 76a6047 commit febb4c1
Show file tree
Hide file tree
Showing 3 changed files with 10 additions and 9 deletions.
4 changes: 2 additions & 2 deletions src/jqLite.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
13 changes: 7 additions & 6 deletions test/jqLiteSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -924,8 +924,8 @@ describe('jqLite', function() {


describe('children', function() {
it('should select non-text children', function() {
var root = jqLite('<div>').html('before-<div></div>after-<span></span>');
it('should only select element nodes', function() {
var root = jqLite('<div><!-- some comment -->before-<div></div>after-<span></span>');
var div = root.find('div');
var span = root.find('span');
expect(root.children()).toJqEqual([div, span]);
Expand All @@ -934,11 +934,12 @@ describe('jqLite', function() {


describe('contents', function() {
it('should select all children nodes', function() {
var root = jqLite('<div>').html('before-<div></div>after-<span></span>');
it('should select all types child nodes', function() {
var root = jqLite('<div><!-- some comment -->before-<div></div>after-<span></span></div>');
var contents = root.contents();
expect(contents.length).toEqual(4);
expect(jqLite(contents[0]).text()).toEqual('before-');
expect(contents.length).toEqual(5);
expect(contents[0].data).toEqual(' some comment ');
expect(contents[1].data).toEqual('before-');
});
});

Expand Down
2 changes: 1 addition & 1 deletion test/testabilityPatch.js
Original file line number Diff line number Diff line change
Expand Up @@ -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]));
}
}
Expand Down

0 comments on commit febb4c1

Please sign in to comment.