Whitespace memleak

[petebacondarwin]

See this discussion: https://groups.google.com/forum/?hl=en-US&fromgroups=#!topic/angular/E01ZrbonZkQ

[petebacondarwin]

While this starts to solve the problem, i.e. empty white space nodes no longer receive a scope, I believe they may well still receive a copy of the controller due to this line in ng-view: https://github.com/angular/angular.js/blob/master/src/ng/directive/ngView.js#L150
This may also need to be changed to ensure that empty white space nodes are ignored.

[IgorMinar]

test is good. but implementation could be more generically done via jqlite

[IgorMinar]

 function JQLiteData(element, key, value) {
+  if (element.nodeType !== 1) return;

that fixes the issue in jqlite but not in jquery

I'd prefer to create a fix that won't put data on text nodes. not attaching data to text nodes is IMO more important than remembering to clean up all this data.

[petebacondarwin]

There are actually very few places where $.data appears:

src/AngularJS,js
This is just the bootstrap, so one may need to ensure that the caller is not trying to bootstrap a text node?

src/jqLite.js
This is in the implementation of jqLiteInheritedData and is readonly

src/src/ng/compile.js
Here, apart from the identified fix in this PR, data is set (to attach a scope) only if the node has a nodeLinkingFn, which does not occur on a text node.

src/ng/directive/ngBind.js
src/ngSanitize/directive/ngBindHtml.js
In all these directives usage is with setting $binding and the node cannot be a text node since it has the directive appearing on it.

src/ng/directive/ngView.js
This needs to be fixed to use children rather than contents to ensure only elements are given controllers.

src/ng/directive/select.js
Only applies data to a parent of an element, which cannot be a text node.

[petebacondarwin]

Updated this PR to provide a more understandable and comprehensive test, and to fix ng-view as well.
This should cover all places where jqLite.data may try to attach something to a text node.
Should fix: #1876 and #1968

[IgorMinar]

otherwise this looks good to me. go ahead and make the final changes, squash and merge to master + v1.0.x

[petebacondarwin]

One problem is do we need to ensure that the scope gets removed from $document or do we never do that?

I.E. I tried this in a unit test:

$document.html('');

which left the jQuery cache with an item on it - i.e. a memory leak?? But I guess that this is not the way to tidy up scope on the document?

[IgorMinar]

turns out that this in fact is not an angular issue but a jquery issue: http://plnkr.co/edit/gWWCom4nnmtA8wKlq39p?p=preview

try jumping between jquery 1.8, 1.9 and angular.element (jqlite) and you'll see that only 1.8 suffers from this issue (as well as from the related IE issue)

[petebacondarwin]

How frustrating! I guess this doesn't change the PR since we still need to support jQuery 1.8?

[IgorMinar]

we'll keep this fix as is right now but we'll likely redo it during the jQuery 1.9 upgrade