fix(tooltip): properly gc timeout on toggle of disabled

- When `disabled` attribute changes, properly garbage collect the timeout Closes #4210 Fixes #4204
angular-ui · Aug 15, 2015 · f8eab55 · f8eab55
1 parent 71e0b8a
commit f8eab55
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 18 deletions.
diff --git a/src/tooltip/test/tooltip.spec.js b/src/tooltip/test/tooltip.spec.js
@@ -246,8 +246,10 @@ describe('tooltip', function() {
   });
 
   describe('with specified popup delay', function() {
-    beforeEach(inject(function($compile) {
-      scope.delay='1000';
+    var $timeout;
+    beforeEach(inject(function($compile, _$timeout_) {
+      $timeout = _$timeout_;
+      scope.delay = '1000';
       elm = $compile(angular.element(
         '<span tooltip="tooltip text" tooltip-popup-delay="{{delay}}" ng-disabled="disabled">Selector Text</span>'
       ))(scope);
@@ -256,22 +258,22 @@ describe('tooltip', function() {
       scope.$digest();
     }));
 
-    it('should open after timeout', inject(function($timeout) {
+    it('should open after timeout', function() {
       elm.trigger('mouseenter');
       expect(tooltipScope.isOpen).toBe(false);
 
       $timeout.flush();
       expect(tooltipScope.isOpen).toBe(true);
-    }));
+    });
 
-    it('should not open if mouseleave before timeout', inject(function($timeout) {
+    it('should not open if mouseleave before timeout', function() {
       elm.trigger('mouseenter');
       expect(tooltipScope.isOpen).toBe(false);
 
       elm.trigger('mouseleave');
       $timeout.flush();
       expect(tooltipScope.isOpen).toBe(false);
-    }));
+    });
 
     it('should use default popup delay if specified delay is not a number', function() {
       scope.delay='text1000';
@@ -280,7 +282,7 @@ describe('tooltip', function() {
       expect(tooltipScope.isOpen).toBe(true);
     });
 
-    it('should not open if disabled is present', inject(function($timeout) {
+    it('should not open if disabled is present', function() {
       elm.trigger('mouseenter');
       expect(tooltipScope.isOpen).toBe(false);
 
@@ -291,10 +293,30 @@ describe('tooltip', function() {
 
       $timeout.flush();
       expect(tooltipScope.isOpen).toBe(false);
-    }));
+    });
+
+    it('should open when not disabled after being disabled - issue #4204', function() {
+      elm.trigger('mouseenter');
+      expect(tooltipScope.isOpen).toBe(false);
+
+      $timeout.flush(500);
+      elmScope.disabled = true;
+      elmScope.$digest();
+
+      $timeout.flush(500);
+      expect(tooltipScope.isOpen).toBe(false);
+
+      elmScope.disabled = false;
+      elmScope.$digest();
+
+      elm.trigger('mouseenter');
+      $timeout.flush();
+
+      expect(tooltipScope.isOpen).toBe(true);
+    });
   });
-  
-  describe( 'with an is-open attribute', function() {
+
+  describe('with an is-open attribute', function() {
     beforeEach(inject(function ($compile) {
       scope.isOpen = false;
       elm = $compile(angular.element(
@@ -304,7 +326,7 @@ describe('tooltip', function() {
       tooltipScope = elmScope.$$childTail;
       scope.$digest();
     }));
-    
+
     it( 'should show and hide with the controller value', function() {
       expect(tooltipScope.isOpen).toBe(false);
       elmScope.isOpen = true;
@@ -314,7 +336,7 @@ describe('tooltip', function() {
       elmScope.$digest();
       expect(tooltipScope.isOpen).toBe(false);
     });
-    
+
     it( 'should update the controller value', function() {
       elm.trigger('mouseenter');
       expect(elmScope.isOpen).toBe(true);
@@ -412,7 +434,7 @@ describe('tooltip', function() {
       elm.trigger('fakeTriggerAttr');
       expect( tooltipScope.isOpen ).toBeFalsy();
     }));
-    
+
     it( 'should not show when trigger is set to "none"', inject(function($compile) {
       elmBody = angular.element(
         '<div><input tooltip="Hello!" tooltip-trigger="none" /></div>'

diff --git a/src/tooltip/tooltip.js b/src/tooltip/tooltip.js
@@ -215,7 +215,7 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.b
               if (isOpenExp) {
                 isOpenExp.assign(ttScope.origScope, ttScope.isOpen);
               }
-              
+
               if (!$rootScope.$$phase) {
                 ttScope.$apply(); // digest required as $apply is not called
               }
@@ -232,7 +232,7 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.b
               if (isOpenExp) {
                 isOpenExp.assign(ttScope.origScope, ttScope.isOpen);
               }
-              
+
               //if tooltip is going to be shown after delay, we must cancel this
               $timeout.cancel(popupTimeout);
               popupTimeout = null;
@@ -269,7 +269,7 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.b
                     hide();
                   }
                 });
-                
+
                 tooltipLinkedScope.$watch(function() {
                   if (!repositionScheduled) {
                     repositionScheduled = true;
@@ -281,7 +281,7 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.b
                     });
                   }
                 });
-                
+
               }
             }
 
@@ -325,6 +325,7 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.b
             attrs.$observe('disabled', function(val) {
               if (popupTimeout && val) {
                 $timeout.cancel(popupTimeout);
+                popupTimeout = null;
               }
 
               if (val && ttScope.isOpen) {
@@ -345,7 +346,7 @@ angular.module('ui.bootstrap.tooltip', ['ui.bootstrap.position', 'ui.bootstrap.b
                 }, 0, false);
               }
             });
-            
+
             if (isOpenExp) {
               scope.$watch(isOpenExp, function(val) {
                 if (val !== ttScope.isOpen) {