Skip to content
This repository has been archived by the owner on May 29, 2019. It is now read-only.

Commit

Permalink
fix(typeahead): dangling event listeners
Browse files Browse the repository at this point in the history
Fixes #4632
Closes #4636
  • Loading branch information
oliversalzburg authored and Foxandxss committed Oct 19, 2015
1 parent 97fd37e commit 94fb282
Show file tree
Hide file tree
Showing 2 changed files with 50 additions and 6 deletions.
46 changes: 40 additions & 6 deletions src/typeahead/test/typeahead.spec.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
describe('typeahead tests', function() {
var $scope, $compile, $document, $templateCache, $timeout;
var $scope, $compile, $document, $templateCache, $timeout, $window;
var changeInputValueTo;

beforeEach(module('ui.bootstrap.typeahead'));
Expand All @@ -25,7 +25,7 @@ describe('typeahead tests', function() {
};
});
}));
beforeEach(inject(function(_$rootScope_, _$compile_, _$document_, _$templateCache_, _$timeout_, $sniffer) {
beforeEach(inject(function(_$rootScope_, _$compile_, _$document_, _$templateCache_, _$timeout_, _$window_, $sniffer) {
$scope = _$rootScope_;
$scope.source = ['foo', 'bar', 'baz'];
$scope.states = [
Expand All @@ -36,6 +36,7 @@ describe('typeahead tests', function() {
$document = _$document_;
$templateCache = _$templateCache_;
$timeout = _$timeout_;
$window = _$window_;
changeInputValueTo = function(element, value) {
var inputEl = findInput(element);
inputEl.val(value);
Expand Down Expand Up @@ -870,6 +871,11 @@ describe('typeahead tests', function() {
});

describe('append to body', function() {
afterEach(function() {
angular.element($window).off('resize');
$document.find('body').off('scroll');
});

it('append typeahead results to body', function() {
var element = prepareInputEl('<div><input ng-model="result" uib-typeahead="item for item in source | filter:$viewValue" typeahead-append-to-body="true"></div>');
changeInputValueTo(element, 'ba');
Expand Down Expand Up @@ -1012,6 +1018,34 @@ describe('typeahead tests', function() {
expect(element).toBeOpenWithActive(3, 0);
});
});

describe('event listeners', function() {
afterEach(function() {
angular.element($window).off('resize');
$document.find('body').off('scroll');
});

it('should register event listeners when attached to body', function() {
spyOn(window, 'addEventListener');
spyOn(document.body, 'addEventListener');

var element = prepareInputEl('<div><input ng-model="result" uib-typeahead="item for item in source | filter:$viewValue" typeahead-append-to-body="true"></div>');

expect(window.addEventListener).toHaveBeenCalledWith('resize', jasmine.any(Function), false);
expect(document.body.addEventListener).toHaveBeenCalledWith('scroll', jasmine.any(Function), false);
});

it('should remove event listeners when attached to body', function() {
spyOn(window, 'removeEventListener');
spyOn(document.body, 'removeEventListener');

var element = prepareInputEl('<div><input ng-model="result" uib-typeahead="item for item in source | filter:$viewValue" typeahead-append-to-body="true"></div>');
$scope.$destroy();

expect(window.removeEventListener).toHaveBeenCalledWith('resize', jasmine.any(Function), false);
expect(document.body.removeEventListener).toHaveBeenCalledWith('scroll', jasmine.any(Function), false);
});
});
});

/* Deprecation tests below */
Expand Down Expand Up @@ -1049,15 +1083,15 @@ describe('typeahead deprecation', function() {
expect($log.warn.calls.argsFor(1)).toEqual(['typeahead is now deprecated. Use uib-typeahead instead.']);
expect($log.warn.calls.argsFor(2)).toEqual(['typeahead-popup is now deprecated. Use uib-typeahead-popup instead.']);
}));

it('should deprecate typeaheadMatch', inject(function($compile, $log, $rootScope, $templateCache, $sniffer){
spyOn($log, 'warn');

var element = '<div typeahead-match index=\"$index\" match=\"match\" query=\"query\" template-url=\"templateUrl\"></div>';
element = $compile(element)($rootScope);
$rootScope.$digest();

expect($log.warn.calls.count()).toBe(1);
expect($log.warn.calls.argsFor(0)).toEqual(['typeahead-match is now deprecated. Use uib-typeahead-match instead.']);
}));
});
});
10 changes: 10 additions & 0 deletions src/typeahead/typeahead.js
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,11 @@ angular.module('ui.bootstrap.typeahead', ['ui.bootstrap.position'])
if (appendToBody || appendToElementId) {
$popup.remove();
}

if (appendToBody) {
angular.element($window).unbind('resize', fireRecalculating);
$document.find('body').unbind('scroll', fireRecalculating);
}
// Prevent jQuery cache memory leak
popUpEl.remove();
});
Expand Down Expand Up @@ -937,6 +942,11 @@ angular.module('ui.bootstrap.typeahead')
if (appendToBody || appendToElementId) {
$popup.remove();
}

if (appendToBody) {
angular.element($window).unbind('resize', fireRecalculating);
$document.find('body').unbind('scroll', fireRecalculating);

This comment has been minimized.

Copy link
@RobJacobs

RobJacobs Oct 20, 2015

Contributor

Just an FYI, we should use 'on' and 'off' instead of 'bind' and 'unbind' which simply call the 'on' and 'off' functions, see here

This comment has been minimized.

Copy link
@Foxandxss

Foxandxss Oct 20, 2015

Contributor

Yes, there are lot of bind / unbind on the code base, but dont worry about them. I am rewriting the whole library to clean it up and those will get changed when I get into them.

}
// Prevent jQuery cache memory leak
popUpEl.remove();
});
Expand Down

0 comments on commit 94fb282

Please sign in to comment.