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

ngSwitch removes DOM elements twice #8662

Closed
matsko opened this issue Aug 18, 2014 · 13 comments
Closed

ngSwitch removes DOM elements twice #8662

matsko opened this issue Aug 18, 2014 · 13 comments

Comments

@matsko
Copy link
Contributor

matsko commented Aug 18, 2014

The code up here:
https://github.com/angular/angular.js/blob/master/src/ng/directive/ngSwitch.js#L150

May seem to contain elements that are called here:
https://github.com/angular/angular.js/blob/master/src/ng/directive/ngSwitch.js#L159

The issue may relate to transclusion blocks that were added in:
ac37915

@btford btford added this to the 1.3.0 milestone Aug 19, 2014
@btford
Copy link
Contributor

btford commented Aug 19, 2014

@matsko is this a regression?

@IgorMinar
Copy link
Contributor

not a regression, but a perf issue.

functionality is not affected, we just do work unnecessarily as far as I can tell.

@IgorMinar IgorMinar modified the milestones: 1.3.0, 1.3.0-rc.0 Aug 25, 2014
@thebigredgeek
Copy link
Contributor

Like this? #8833

@caitp
Copy link
Contributor

caitp commented Aug 29, 2014

The referenced sha specifically fixes an issue where DOM nodes were attempted to be removed twice (resulting in a type error), but it's quite likely that there are more problems with it

@thebigredgeek
Copy link
Contributor

Part of the problem is that the callback for the $animate.leave will always reference the last value of i in that loop. There is a closure issue there.

@IgorMinar
Copy link
Contributor

I just spoke with @matsko about this one. there is one test that fails if the direct removal is removed (:)).

we think the test is no longer valid or was never valid and @matsko is going to handle that.

@IgorMinar
Copy link
Contributor

I don't think we need the callback at all

@thebigredgeek
Copy link
Contributor

Hmmm, so we should keep the complete drain but remove the selected callback drain for $animate.leave

@thebigredgeek
Copy link
Contributor

Also, just this morning getting this when I try to package :(

Running "minall" task

Exception in thread "main" java.lang.UnsupportedClassVersionError: com/google/javascript/jscomp/CommandLineRunner : Unsupported major.minor version 51.0
at java.lang.ClassLoader.defineClass1(Native Method)
at java.lang.ClassLoader.defineClassCond(ClassLoader.java:637)
at java.lang.ClassLoader.defineClass(ClassLoader.java:621)
at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:141)
at java.net.URLClassLoader.defineClass(URLClassLoader.java:283)
at java.net.URLClassLoader.access$000(URLClassLoader.java:58)
at java.net.URLClassLoader$1.run(URLClassLoader.java:197)
at java.security.AccessController.doPrivileged(Native Method)
at java.net.URLClassLoader.findClass(URLClassLoader.java:190)
at java.lang.ClassLoader.loadClass(ClassLoader.java:306)
at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:301)
at java.lang.ClassLoader.loadClass(ClassLoader.java:247)
at java.lang.ClassLoader.defineClass1(Native Method)
at java.lang.ClassLoader.defineClassCond(ClassLoader.java:637)
at java.lang.ClassLoader.defineClass(ClassLoader.java:621)
at java.security.SecureClassLoader.defineClass(SecureClassLoader.java:141)
at java.net.URLClassLoader.defineClass(URLClassLoader.java:283)
at java.net.URLClassLoader.access$000(URLClassLoader.java:58)
at java.net.URLClassLoader$1.run(URLClassLoader.java:197)
at java.security.AccessController.doPrivileged(Native Method)
at java.net.URLClassLoader.findClass(URLClassLoader.java:190)
at java.lang.ClassLoader.loadClass(ClassLoader.java:306)
at sun.misc.Launcher$AppClassLoader.loadClass(Launcher.java:301)
at java.lang.ClassLoader.loadClass(ClassLoader.java:247)

@IgorMinar
Copy link
Contributor

you need java7

@thebigredgeek
Copy link
Contributor

Oh okay yes I see ;)

@matsko
Copy link
Contributor Author

matsko commented Aug 29, 2014

@thebigredgeek this was fixed using $animate.cancel.

@matsko
Copy link
Contributor Author

matsko commented Aug 29, 2014

c9b0bfe

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.

5 participants