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

[ovaillancourt]

Hi!

After upgrading to angular 1.2.16, we noticed a regression in the ngSwitch directive that seems to have been present since 1.2.14 and is also still present in 1.3.0 beta-7.

The issue occurs when two elements have the same ng-switch-when value. When toggling between values of the switch expression, angular will throw the following error when it tries to "hide" the elements having the same ng-switch-when.

The call stack produced by the error:

TypeError: Cannot read property 'remove' of undefined
    at Object.ngSwitchWatchAction [as fn] (https://code.angularjs.org/1.2.14/angular.js:20225:34)
    at Scope.$digest (https://code.angularjs.org/1.2.14/angular.js:11930:29)
    at Scope.$apply (https://code.angularjs.org/1.2.14/angular.js:12179:24)
    at HTMLAnchorElement.<anonymous> (https://code.angularjs.org/1.2.14/angular.js:18228:21)
    at https://code.angularjs.org/1.2.14/angular.js:2673:10
    at forEach (https://code.angularjs.org/1.2.14/angular.js:329:20)
    at HTMLAnchorElement.eventHandler (https://code.angularjs.org/1.2.14/angular.js:2672:5) angular.js:9509
(anonymous function) angular.js:9509
(anonymous function) angular.js:6950
Scope.$digest angular.js:11949
Scope.$apply angular.js:12179
(anonymous function) angular.js:18228
(anonymous function) angular.js:2673
forEach angular.js:329
eventHandler angular.js:2672

Reproduction:

Here is a js fiddle that reproduces this issue: http://jsfiddle.net/QZkZN/4/
(Click on "show edit", then "cancel", then "show edit" again to trigger the error.)

Reproducibility:

The bug is 100% reproductible on chrome latest - we haven't tried other browsers.

Source of the regression:

This line and the surrounding code introduced in commit e988199 (changes to file: src/ng/directive/ngSwitch.js) is causing the error:

angular.js/src/ng/directive/ngSwitch.js

Line 149 in e988199

previousElements[i].remove();

[caitp]

This does look like a regression, I'll see if I can put together a patch tomorrow morning if noone beats me to it

[caitp]

Actually, you know what? I'm not sure the way it's being used in that plunkr was ever really supported. So maybe we shouldn't consider this a regression.

Yeah, I'm not sure about this.

[caitp]

hmm.. but we do have an array of $transclude functions for each case label, so I guess this is supported... yeah, it's a regression, okay... weird that we don't have tests for it!

[ovaillancourt]

Thanks for the quick follow-up!

[tmundt]

If come around this issue when using ng-switch within a form and a XHTML-page. It concerns 1.2.14 and greater and 1.3.beta7 (same error message)

The error being thrown in Chrome looks like this (Version 34.0.1847.131 m):

Error: previousElements[i] is undefined
ngSwitchWatchAction@http://cdnjs.cloudflare.com/ajax/libs/angular.js/1.3.0-beta.7/angular.js:21772
$RootScopeProvider/this.$get</Scope.prototype.$digest@http://cdnjs.cloudflare.com/ajax/libs/angular.js/1.3.0-beta.7/angular.js:12558
$RootScopeProvider/this.$get</Scope.prototype.$apply@http://cdnjs.cloudflare.com/ajax/libs/angular.js/1.3.0-beta.7/angular.js:12823
radioInputType/listener@http://cdnjs.cloudflare.com/ajax/libs/angular.js/1.3.0-beta.7/angular.js:17698
please see an code-snippet:

   <form class="form-horizontal" role="form" ng-controller="formRegistrationTab3Controller" ng-switch="paySelect" name="cashForm">
        <div id="tabform3">
            <!-- Bankverbindungs-Auswahl --> 
            <div class="form-group">
                <label class="col-sm-3 control-label">Bezahlinformationen</label> 
                <div class="col-sm-9">
                    <label><input type="radio" ng-model="paySelect" value="DirectDebiting" ng-change="updateCount();" /> Bankeinzug</label> <br />
                    <label><input type="radio" ng-model="paySelect" value="CreditCard" ng-change="updateCount();" /> Kreditkarte</label> <br />
                    <label><input type="radio" ng-model="paySelect" value="DecideLater" ng-change="updateCount();" /> Später angeben</label>
                </div>
            </div>

[caitp]

Wait, this was added to v1.2.x also... So yeah, you're using old versions, @tmundt, that's all. Try with the latest 1.3 revision, it should work for you (and if it doesn't, please file a bug)

[tmundt]

It works until version 1.2.13 and not after that (that is 1.2.14 to 1.2.16) and 1.3beta7 (the newest version). I have seen that you did make a PR/fix, is that right?

[caitp]

@tmundt yes, I merged the PR last week, this should be fixed

[tmundt]

@caitp ok, thank you! That is weird somehow. Must have a closer look on that.

[tmundt]

Why do you assign i and ii to selectedScopes.length and destroys the i within the for-next loop?

scope.$watch(watchExpr, function ngSwitchWatchAction(value) {
    var i, ii = selectedScopes.length;
    if(ii > 0) {
      if(previousElements) {
        for (i = 0; i < ii; i++) {
          previousElements[i].remove();
        }
        previousElements = null;
      }

[caitp]

You're looking at old pre-fix code, dude!

[tmundt]

Ups... this was from the CDN-Link to version 1.3beta7 starting at line 21767... so they aren't updated? I just called it up @ //cdnjs.cloudflare.com/ajax/libs/angular.js/1.3.0-beta.7/angular.js

[caitp]

http://ajax.googleapis.com/ajax/libs/angularjs/1.2.16/angular.js for instance

[tmundt]

Ok that is the same code starting at line 20619. And I tested also the other versions 1.2.15 and 1.2.14 also.

[caitp]

Yeah, you're right. http://ajax.googleapis.com/ajax/libs/angularjs/1.3.0-beta.8/angular.js

 for (i = 0, ii = previousElements.length; i < ii; ++i) {
   previousElements[i].remove();
 }
 previousElements.length = 0;

 for (i = 0, ii = selectedScopes.length; i < ii; ++i) {
   var selected = selectedElements[i];
   selectedScopes[i].$destroy();
   previousElements[i] = selected;
   $animate.leave(selected, function() {
     previousElements.splice(i, 1);
   });
 }
 // ...

[caitp]

There hasn't been a 1.2.x release in the past few weeks, so this is only in the 1.3 channel for now

[tmundt]

Oh ok. The code for version 1.3beta7 makes sense and my code is working after switching to your URL (and not using //cdnjs.cloudflare.com/ajax/libs/angular.js/1.3.0-beta.7/angular.js). Hope that your fix will be in the 1.2.x channel soon.
Thanks a lot, great work!