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

Angular 1.0.3 breaks directives using ng-transclude #1627

Closed
Addeventure opened this issue Nov 29, 2012 · 22 comments
Closed

Angular 1.0.3 breaks directives using ng-transclude #1627

Addeventure opened this issue Nov 29, 2012 · 22 comments

Comments

@Addeventure
Copy link

ng-translude for directives is broken in angular 1.0.3. It seems to be related with the fix for issue #1313

Changing view using $location from a view that uses a directive with transcluded content throws:
TypeError: Cannot read property '$$childHead' of null

Take a look at this simple snippet:
http://jsfiddle.net/JJt3u/

Snippet fails in Chrome 23.0.1271.64, and Firefox 17.0

@petebacondarwin
Copy link
Member

This issue is not caused by that change, since the changed line does not get called before the exception is thrown.

The compiler is creating an extra scope (005 on my machine) that is not attached to a parent, it is the destruction of this that is failing. This extra scope is the one that is created if your directive template has more than one top level DOM node. (In the case of your fiddle it is the sneaky bit of whitespace between the div and the button.)

Here is a Plunker http://plnkr.co/edit/aSN1H4?p=preview. If you change which template is being used you will get different success or exception.

@petebacondarwin
Copy link
Member

Possibly due to this commit?
4e45a2f#L0L709

@Addeventure
Copy link
Author

Ouch, yes that was a sneaky whitespace. Thank you for looking in to this! Just to clarify, it is not the directive template but the transcluded DOM which have more than one top node.

I'm surprised that I am the only one that have reported this issue (although 1.0.3 was released quite recently) as it feels like a very normal use case of directives.

One more clarification/question: Having more than one top node in transcluded dom is supposed to be supported right? (Otherwise I have a lot of templates to rewrite ;))

@petebacondarwin
Copy link
Member

Yes, sorry, top level nodes in the transcluded DOM. Multi top level nodes
are supported - AngularJS just wraps them in a to make a single top
level node.

On 29 November 2012 13:31, Andreas Duchen notifications@github.com wrote:

Ouch, yes that was a sneaky whitespace. Thank you for looking in to this!
Just to clarify, it is not the directive template but the transcluded DOM
which have more than one top node.

I'm surprised that I am the only one that have reported this issue
(although 1.0.3 was released quite recently) as it feels like a very normal
use case of directives.

One more clarification/question: Having more than one top node in
transcluded dom is supposed to be supported right? (Otherwise I have a
lot of templates to rewrite ;))


Reply to this email directly or view it on GitHubhttps://github.com//issues/1627#issuecomment-10847094.

@qualidafial
Copy link

I'm having this issue as well.

@murilobr
Copy link
Contributor

Fast solution commenting lines 7808 and 7809

Comment this:
this.$parent = this.$$nextSibling = this.$$prevSibling = this.$$childHead =
this.$$childTail = null;

Add this:
this.$$nextSibling = null;

@pkozlowski-opensource
Copy link
Member

Happens also without jQuery: http://jsfiddle.net/JJt3u/3/

@ggoodman
Copy link
Contributor

Also broken in 1.1.1

/edited from 1.1.0 (thanks @pkozlowski-opensource)

@pkozlowski-opensource
Copy link
Member

@ggoodman 1.1.0 or 1.1.1?

@petebacondarwin
Copy link
Member

@murilobr - You are right - commenting out those lines does fix the problem: http://plnkr.co/edit/IOeW5Q?p=preview

@dbinit
Copy link
Contributor

dbinit commented Nov 29, 2012

I'm seeing this too.

From what I can tell, it looks like every top level element in transcluded content (including whitespace) is being assigned the same scope (stored in data()).

This causes $destroy to be fired more than once on the scope when the elements are removed.

It's possible that this is an existing bug with $compile that just surfaced because of the change to null out $parent on $destroy.

@dbinit
Copy link
Contributor

dbinit commented Nov 29, 2012

Changing that line from:

this.$parent = this.$$nextSibling = this.$$prevSibling = this.$$childHead =
this.$$childTail = null;

to:

this.$$nextSibling = this.$$prevSibling = this.$$childHead = this.$$childTail = null;

Also seems to fix the problem.

Or, perhaps a better solution, you can add:

if (!parent) return;

right below this in that same function:

var parent = this.$parent;

@ghost ghost assigned IgorMinar Nov 29, 2012
@petebacondarwin
Copy link
Member

One for Igor, I think!

@IgorMinar
Copy link
Contributor

ok, I'm on this. The problem is due to bd524fc which surfaced a bug in destruction of a multi-rooted DOM with a single scope associated with all of the roots. we mistakinly destroy the shared scope onces for each root, which is wrong. once a scope is destroyed, it's destroyed. I'll work on a fix for this.

IgorMinar added a commit to IgorMinar/angular.js that referenced this issue Nov 30, 2012
Due to bd524fc calling $destroy() on a scope mupltiple times cases NPE.

Closes angular#1627
IgorMinar added a commit that referenced this issue Nov 30, 2012
Due to bd524fc calling $destroy() on a scope mupltiple times cases NPE.

Closes #1627
@leefernandes
Copy link
Contributor

Thanks for the fixes in here. You guys are savers of lives, or the time wherein one invests it.

@robertjchristian
Copy link

Is the fix rolled into a release? I am seeing this issue in conjunction with angular-ui and Angular 1.0.3 (from CDN). It appears after transitioning from a partial using the accordian directive.

I wrote it up on SO: http://stackoverflow.com/questions/14285932/angular-js-bootstrap-accordian-example#comment19838051_14285932

@pkozlowski-opensource
Copy link
Member

@robertjchristian it was fixed in master but not yet released... What you can do is to take the snapshot of the bug-fix release (to be 1.0.4) from http://code.angularjs.org/snapshot/. This one should be rather stable (it has only bug and documentation fixes)

@robertjchristian
Copy link

That did the trick.  Thank you.Pawel Kozlowski notifications@github.com wrote:@robertjchristian it was fixed in master but not yet released... What you can do is to take the snapshot of the bug-fix release (to be 1.0.4) from http://code.angularjs.org/snapshot/. This one should be rather stable (it has only bug and documentation fixes)


Reply to this email directly or view it on GitHub.

jbdeboer pushed a commit to jbdeboer/angular.js that referenced this issue Jan 17, 2013
Due to bd524fc calling $destroy() on a scope mupltiple times cases NPE.

Closes angular#1627
@munsellj
Copy link

Sorry to drudge up an old issue but would really appreciate if anyone has some insight. I'm trying to update an app from v1.0.2 to v1.0.6 and am pretty sure this change is causing an issue I'm seeing. Basically an NPE gets thrown when destroying a scope upon receiving an event broadcast. I think this happens because the broadcast tries to execute again on a scope that no longer exists and the broadcast can't be cancelled.

The error is something like this in Chrome:

TypeError: Cannot read property '$$nextSibling' of null
    at Object.Scope.$broadcast (https://ajax.googleapis.com/ajax/libs/angularjs/1.0.6/angular.js:8271:57)
    at Object.$scope.broadcastDone (http://run.plnkr.co/t0qjPOtC0B8TnSFG/app.js:7:12)
    at https://ajax.googleapis.com/ajax/libs/angularjs/1.0.6/angular.js:6315:19
    at https://ajax.googleapis.com/ajax/libs/angularjs/1.0.6/angular.js:12903:13
    at Object.Scope.$eval (https://ajax.googleapis.com/ajax/libs/angularjs/1.0.6/angular.js:8011:28)
    at Object.Scope.$apply (https://ajax.googleapis.com/ajax/libs/angularjs/1.0.6/angular.js:8091:23)
    at HTMLAnchorElement.<anonymous> (https://ajax.googleapis.com/ajax/libs/angularjs/1.0.6/angular.js:12902:17)
    at https://ajax.googleapis.com/ajax/libs/angularjs/1.0.6/angular.js:1972:10
    at Array.forEach (native)
    at forEach (https://ajax.googleapis.com/ajax/libs/angularjs/1.0.6/angular.js:131:11) angular.js:5704

Here's a basic plunkr that demonstrates the problem(check the console to see the error, click the 'Done' link to trigger): http://plnkr.co/edit/zAUL4WliHpDp9c2K4xNo?p=preview

If you update the angular version to 1.0.2 in index.html it will work as it did for us previously, but starting with v1.0.3 we're see the error. It happens both in the latest versions of Chrome/Canary, Firefox and Safari so shouldn't be anything browser specific.

Not sure if this is the result of a bug or bad practice, if the later it would be awesome if someone could suggest an alternate implementation :-)

@cm325
Copy link

cm325 commented Oct 25, 2013

👍 even in 1.1.5

@manikantag
Copy link

Issue still coming in 1.3.0.beta-18 too. I m basically destroying Angular-Strap modal window and calling a $http request after that it is destroyed.

@IgorMinar
Copy link
Contributor

That's likely a different issue. Please file a bug with minimal reproduction.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.