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

fix(ngSwitch): properly support case labels with different numbers of transclude fns #7373

Closed
wants to merge 1 commit into from

Conversation

caitp
Copy link
Contributor

@caitp caitp commented May 7, 2014

Due to a regression introduced several releases ago, the ability for multiple
transclude functions to work correctly changed, as they would break if
different case labels had different numbers of transclude functions.

This CL corrects this by not assuming that previous elements and scope count
have the same length.

Closes #7372

@caitp caitp added this to the 1.3.0 milestone May 7, 2014
previousElements[i].remove();
}
previousElements = null;
previousElements.length = 0;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't related to the change, but I didn't want to be creating new containers all the time, might get a little perf boost out of that.

… transclude fns

Due to a regression introduced several releases ago, the ability for multiple transclude functions
to work correctly changed, as they would break if different case labels had different numbers of
transclude functions.

This CL corrects this by not assuming that previous elements and scope count have the same length.
@caitp
Copy link
Contributor Author

caitp commented May 7, 2014

The bug is sort of hard to explain, basically, you can have multiple ng-switch-when children using the same case label, so when you have one case label with 2 element, and another case label with only one element, then we get a reference error dealing with previousElements.

So the fix is just to make sure that the collections are iterated over correctly, because previousElements.length does not always match selectedScopes.length

The other changes (= [] to .length = 0) are not really related to the CL, but can perform better in some browsers due to not allocating a new JSObject, and not necessarily touching the heap at all.

@IgorMinar
Copy link
Contributor

lgtm

@caitp caitp closed this in ac37915 May 8, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ngSwitch regression 1.2.14 and up - crash in ng-switch with multiple ng-switch-when using the same term.
2 participants