chore: ensure tests are gc'd

- Ensure event listeners globally on service instantiation are garbage collected properly Closes #5017 Fixes #4996
angular-ui · Dec 5, 2015 · 8930a7f · 8930a7f
1 parent 8a7ebbf
commit 8930a7f
Show file tree

Hide file tree

Showing 9 changed files with 55 additions and 11 deletions.
diff --git a/src/modal/modal.js b/src/modal/modal.js
@@ -331,7 +331,13 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.stackedMap'])
         }
       }
 
-      $document.bind('keydown', function(evt) {
+      $document.on('keydown', keydownListener);
+
+      $rootScope.$on('$destroy', function() {
+        $document.off('keydown', keydownListener);
+      });
+
+      function keydownListener(evt) {
         if (evt.isDefaultPrevented()) {
           return evt;
         }
@@ -369,7 +375,7 @@ angular.module('ui.bootstrap.modal', ['ui.bootstrap.stackedMap'])
             }
           }
         }
-      });
+      }
 
       $modalStack.open = function(modalInstance, modal) {
         var modalOpener = $document[0].activeElement,

diff --git a/src/modal/test/modal.spec.js b/src/modal/test/modal.spec.js
@@ -154,6 +154,7 @@ describe('$uibModal', function () {
     body.find('div.modal').remove();
     body.find('div.modal-backdrop').remove();
     body.removeClass('modal-open');
+    $document.off('keydown');
   });
 
   function triggerKeyDown(element, keyCode, shiftKey) {

diff --git a/src/popover/test/popover-html.spec.js b/src/popover/test/popover-html.spec.js
@@ -11,7 +11,8 @@ describe('popover', function() {
   // load the template
   beforeEach(module('uib/template/popover/popover-html.html'));
 
-  beforeEach(inject(function($rootScope, $compile, $sce) {
+  beforeEach(inject(function($rootScope, $compile, $sce, _$document_) {
+    $document = _$document_;
     elmBody = angular.element(
       '<div><span uib-popover-html="template">Selector Text</span></div>'
     );
@@ -25,6 +26,10 @@ describe('popover', function() {
     tooltipScope = elmScope.$$childTail;
   }));
 
+  afterEach(function() {
+    $document.off('keypress');
+  });
+
   it('should not be open initially', inject(function() {
     expect(tooltipScope.isOpen).toBe(false);
 

diff --git a/src/popover/test/popover-template.spec.js b/src/popover/test/popover-template.spec.js
@@ -3,7 +3,8 @@ describe('popover template', function() {
       elmBody,
       scope,
       elmScope,
-      tooltipScope;
+      tooltipScope,
+      $document;
 
   // load the popover code
   beforeEach(module('ui.bootstrap.popover'));
@@ -16,7 +17,8 @@ describe('popover template', function() {
     $templateCache.put('myUrl', [200, '<span>{{ myTemplateText }}</span>', {}]);
   }));
 
-  beforeEach(inject(function($rootScope, $compile) {
+  beforeEach(inject(function($rootScope, $compile, _$document_) {
+    $document = _$document_;
     elmBody = angular.element(
       '<div><span uib-popover-template="templateUrl">Selector Text</span></div>'
     );
@@ -31,6 +33,10 @@ describe('popover template', function() {
     tooltipScope = elmScope.$$childTail;
   }));
 
+  afterEach(function() {
+    $document.off('keypress');
+  });
+
   it('should open on click', inject(function() {
     elm.trigger('click');
     tooltipScope.$digest();

diff --git a/src/popover/test/popover.spec.js b/src/popover/test/popover.spec.js
@@ -3,15 +3,17 @@ describe('popover', function() {
       elmBody,
       scope,
       elmScope,
-      tooltipScope;
+      tooltipScope,
+      $document;
 
   // load the popover code
   beforeEach(module('ui.bootstrap.popover'));
 
   // load the template
   beforeEach(module('uib/template/popover/popover.html'));
 
-  beforeEach(inject(function($rootScope, $compile) {
+  beforeEach(inject(function($rootScope, $compile, _$document_) {
+    $document = _$document_;
     elmBody = angular.element(
       '<div><span uib-popover="popover text">Selector Text</span></div>'
     );
@@ -24,6 +26,10 @@ describe('popover', function() {
     tooltipScope = elmScope.$$childTail;
   }));
 
+  afterEach(function() {
+    $document.off('keypress');
+  });
+
   it('should not be open initially', inject(function() {
     expect(tooltipScope.isOpen).toBe(false);
 

diff --git a/src/tooltip/test/tooltip-template.spec.js b/src/tooltip/test/tooltip-template.spec.js
@@ -3,7 +3,8 @@ describe('tooltip template', function() {
       elmBody,
       scope,
       elmScope,
-      tooltipScope;
+      tooltipScope,
+      $document;
 
   // load the popover code
   beforeEach(module('ui.bootstrap.tooltip'));
@@ -15,7 +16,8 @@ describe('tooltip template', function() {
     $templateCache.put('myUrl', [200, '<span>{{ myTemplateText }}</span>', {}]);
   }));
 
-  beforeEach(inject(function($rootScope, $compile) {
+  beforeEach(inject(function($rootScope, $compile, _$document_) {
+    $document = _$document_;
     elmBody = angular.element(
       '<div><span uib-tooltip-template="templateUrl">Selector Text</span></div>'
     );
@@ -30,6 +32,10 @@ describe('tooltip template', function() {
     tooltipScope = elmScope.$$childTail;
   }));
 
+  afterEach(function() {
+    $document.off('keypress');
+  });
+
   function trigger(element, evt) {
     evt = new Event(evt);
 

diff --git a/src/tooltip/test/tooltip.spec.js b/src/tooltip/test/tooltip.spec.js
@@ -26,6 +26,10 @@ describe('tooltip', function() {
     tooltipScope = elmScope.$$childTail;
   }));
 
+  afterEach(function() {
+    $document.off('keypress');
+  });
+
   function trigger(element, evt) {
     evt = new Event(evt);
 

diff --git a/src/tooltip/test/tooltip2.spec.js b/src/tooltip/test/tooltip2.spec.js
@@ -37,6 +37,10 @@ describe('tooltip directive', function() {
     });
   });
 
+  afterEach(function() {
+    $document.off('keypress');
+  });
+
   function compileTooltip(ttipMarkup) {
     var fragment = $compile('<div>' + ttipMarkup + '</div>')($rootScope);
     $rootScope.$digest();

diff --git a/src/tooltip/tooltip.js b/src/tooltip/tooltip.js
@@ -71,7 +71,13 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.s
    */
   this.$get = ['$window', '$compile', '$timeout', '$document', '$uibPosition', '$interpolate', '$rootScope', '$parse', '$$stackedMap', function($window, $compile, $timeout, $document, $position, $interpolate, $rootScope, $parse, $$stackedMap) {
     var openedTooltips = $$stackedMap.createNew();
-    $document.on('keypress', function(e) {
+    $document.on('keypress', keypressListener);
+
+    $rootScope.$on('$destroy', function() {
+      $document.off('keypress', keypressListener);
+    });
+
+    function keypressListener(e) {
       if (e.which === 27) {
         var last = openedTooltips.top();
         if (last) {
@@ -80,7 +86,7 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.s
           last = null;
         }
       }
-    });
+    }
 
     return function $tooltip(ttType, prefix, defaultTriggerShow, options) {
       options = angular.extend({}, defaultOptions, globalOptions, options);