Merge pull request #155 from thgreasi/master

fix(sortable): race condition when DOM changes inside a digest loop.

Author: thgreasi

bower.json

@@ -1,6 +1,6 @@
 {
   "name": "angular-ui-sortable",
-  "version": "0.12.2",
+  "version": "0.12.3",
   "description": "This directive allows you to jQueryUI Sortable.",
   "author": "https://github.com/angular-ui/ui-sortable/graphs/contributors",
   "license": "MIT",

package.json

@@ -1,6 +1,6 @@
 {
   "name": "angular-ui-sortable",
-  "version": "0.12.2",
+  "version": "0.12.3",
   "description": "This directive allows you to jQueryUI Sortable.",
   "author": "https://github.com/angular-ui/ui-sortable/graphs/contributors",
   "license": "MIT",

src/sortable.js

@@ -42,6 +42,8 @@ angular.module('ui.sortable', [])
             scope.$watch(attrs.ngModel+'.length', function() {
               // Timeout to let ng-repeat modify the DOM
               $timeout(function() {
+                // ensure that the jquery-ui-sortable widget instance
+                // is still bound to the directive's element
                 if (!!element.data('ui-sortable')) {
                   element.sortable('refresh');
                 }
@@ -174,18 +176,23 @@ angular.module('ui.sortable', [])
             };
 
             scope.$watch(attrs.uiSortable, function(newVal /*, oldVal*/) {
-              angular.forEach(newVal, function(value, key) {
-                if(callbacks[key]) {
-                  if( key === 'stop' ){
-                    // call apply after stop
-                    value = combineCallbacks(
-                      value, function() { scope.$apply(); });
+              // ensure that the jquery-ui-sortable widget instance
+              // is still bound to the directive's element
+              if (!!element.data('ui-sortable')) {
+                angular.forEach(newVal, function(value, key) {
+                  if(callbacks[key]) {
+                    if( key === 'stop' ){
+                      // call apply after stop
+                      value = combineCallbacks(
+                        value, function() { scope.$apply(); });
+                    }
+                    // wrap the callback
+                    value = combineCallbacks(callbacks[key], value);
                   }
-                  // wrap the callback
-                  value = combineCallbacks(callbacks[key], value);
-                }
-                element.sortable('option', key, value);
-              });
+                  
+                  element.sortable('option', key, value);
+                });
+              }
             }, true);
 
             angular.forEach(callbacks, function(value, key) {

test/sortable.spec.js

@@ -46,6 +46,27 @@ describe('uiSortable', function() {
       });
     });
 
+    it('should not try to apply options to a destroyed sortable', function() {
+      inject(function($compile, $rootScope, $timeout) {
+        var element;
+        var childScope = $rootScope.$new();
+        element = $compile('<div><ul ui-sortable="opts" ng-model="items"><li ng-repeat="item in items">{{ item }}</li></ul></div>')(childScope);
+        $rootScope.$apply(function() {
+          childScope.items = ['One', 'Two', 'Three'];
+          childScope.opts = {
+            update: function() {}
+          };
+
+          element.remove(element.firstChild);
+        });
+
+        expect(function() {
+          $timeout.flush();
+        }).not.toThrow();
+
+      });
+    });
+
   });
 
 });