Commit 55d82623 authored by Robert Knight's avatar Robert Knight

Clear pending timeouts when AppController is destroyed

Fix a flaky AppController test and potential causes of flakyness or
runtime errors in other tests due to timeouts set up in directives or
controllers firing after the associated scope is destroyed.

Add a helper which sets a timeout that is cleared automatically
when an associated scope is destroyed.

In the AppController test this manifested itself with a timeout
being called when `$document[0]` was not set.
parent 47f19feb
......@@ -6,6 +6,7 @@ var scrollIntoView = require('scroll-into-view');
var annotationMetadata = require('./annotation-metadata');
var events = require('./events');
var parseAccountID = require('./filter/persona').parseAccountID;
var scopeTimeout = require('./util/scope-timeout');
function authStateFromUserID(userid) {
if (userid) {
......@@ -107,7 +108,7 @@ module.exports = function AppController(
function scrollToView(selector) {
// Add a timeout so that if the element has just been shown (eg. via ngIf)
// it is added to the DOM before we try to locate and scroll to it.
setTimeout(function () {
scopeTimeout($scope, function () {
scrollIntoView($document[0].querySelector(selector));
}, 0);
}
......
......@@ -2,6 +2,8 @@
var angular = require('angular');
var scopeTimeout = require('../util/scope-timeout');
module.exports = function () {
return {
bindToController: true,
......@@ -15,7 +17,7 @@ module.exports = function () {
$scope.$watch('vm.isOpen', function (isOpen) {
if (isOpen) {
// Focus the input and select it once the dialog has become visible
setTimeout(function () {
scopeTimeout($scope, function () {
shareLinkInput.focus();
shareLinkInput.select();
});
......@@ -32,11 +34,9 @@ module.exports = function () {
// Stop listening for clicks outside the dialog once it is closed.
// The setTimeout() here is to ignore the initial click that opens
// the dialog.
setTimeout(function () {
document.addEventListener('click', hideListener);
},
0
);
scopeTimeout($scope, function () {
document.addEventListener('click', hideListener);
}, 0);
}
}.bind(this));
......
......@@ -6,6 +6,7 @@ var debounce = require('lodash.debounce');
var commands = require('../markdown-commands');
var mediaEmbedder = require('../media-embedder');
var renderMarkdown = require('../render-markdown');
var scopeTimeout = require('../util/scope-timeout');
/**
* @ngdoc directive
......@@ -49,7 +50,7 @@ module.exports = function($sanitize) {
// A timeout is used so that focus() is not called until
// the visibility change has been applied (by adding or removing
// the relevant CSS classes)
setTimeout(function () {
scopeTimeout(scope, function () {
input.focus();
}, 0);
}
......
'use strict';
/**
* Sets a timeout which is linked to the lifetime of an Angular scope.
*
* When the scope is destroyed, the timeout will be cleared if it has
* not already fired.
*
* The callback is not invoked within a $scope.$apply() context. It is up
* to the caller to do that if necessary.
*
* @param {Scope} $scope - An Angular scope
* @param {Function} fn - Callback to invoke with setTimeout
* @param {number} delay - Delay argument to pass to setTimeout
*/
module.exports = function ($scope, fn, delay, setTimeoutFn, clearTimeoutFn) {
setTimeoutFn = setTimeoutFn || setTimeout;
clearTimeoutFn = clearTimeoutFn || clearTimeout;
var removeDestroyHandler;
var id = setTimeoutFn(function () {
removeDestroyHandler();
fn();
}, delay);
removeDestroyHandler = $scope.$on('$destroy', function () {
clearTimeoutFn(id);
});
};
'use strict';
var scopeTimeout = require('../scope-timeout');
function FakeScope() {
this.listeners = {};
this.$on = function (event, fn) {
this.listeners[event] = this.listeners[event] || [];
this.listeners[event].push(fn);
return function () {
this.listeners[event] = this.listeners[event].filter(function (otherFn) {
return otherFn !== fn;
});
}.bind(this);
};
}
describe('scope-timeout', function () {
var fakeSetTimeout;
var fakeClearTimeout;
beforeEach(function () {
fakeSetTimeout = sinon.stub().returns(42);
fakeClearTimeout = sinon.stub();
});
it('schedules a timeout', function () {
var $scope = new FakeScope();
var callback = sinon.stub();
scopeTimeout($scope, callback, 0, fakeSetTimeout, fakeClearTimeout);
assert.calledOnce(fakeSetTimeout);
var timeoutFn = fakeSetTimeout.args[0][0];
timeoutFn();
assert.called(callback);
});
it('removes the scope listener when the timeout fires', function () {
var $scope = new FakeScope();
var callback = sinon.stub();
scopeTimeout($scope, callback, 0, fakeSetTimeout, fakeClearTimeout);
assert.equal($scope.listeners.$destroy.length, 1);
var timeoutFn = fakeSetTimeout.args[0][0];
timeoutFn();
assert.equal($scope.listeners.$destroy.length, 0);
});
it('clears the timeout when the scope is destroyed', function () {
var $scope = new FakeScope();
var callback = sinon.stub();
scopeTimeout($scope, callback, 0, fakeSetTimeout, fakeClearTimeout);
var destroyFn = $scope.listeners.$destroy[0];
destroyFn();
assert.calledWith(fakeClearTimeout, 42);
});
});
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment