From 4ccb141a425777490e7537d0cea11e0232383f69 Mon Sep 17 00:00:00 2001 From: Luke Murray Date: Wed, 7 Aug 2013 10:54:20 +1000 Subject: [PATCH 1/4] Add a destroy function on the viewer to clean up and remove elements created by open seadragon. Add removeAllHandlersForAllEvents to clean up all events on destroy. Clear the onDraw callback on Overlay destroy. --- src/eventhandler.js | 13 ++++++++++++- src/overlay.js | 3 +++ src/viewer.js | 32 ++++++++++++++++++++++++++++++++ 3 files changed, 47 insertions(+), 1 deletion(-) diff --git a/src/eventhandler.js b/src/eventhandler.js index e265fbf7..430c23fc 100644 --- a/src/eventhandler.js +++ b/src/eventhandler.js @@ -95,11 +95,22 @@ $.EventHandler.prototype = { * @function * @param {String} eventName - Name of event for which all handlers are to be removed. */ - removeAllHandlers: function( eventName ){ + removeAllHandlers: function( eventName ) { this.events[ eventName ] = []; }, + /** + * Remove every event handler for all event types + * @function + */ + removeAllHandlersForAllEvents: function( ) { + for (var eventType in this.events) { + this.events[eventType] = []; + } + }, + + /** * Retrive the list of all handlers registered for a given event. * @function diff --git a/src/overlay.js b/src/overlay.js index 8635499f..a0e40490 100644 --- a/src/overlay.js +++ b/src/overlay.js @@ -157,6 +157,9 @@ } } + // clear the onDraw callback + this.onDraw = null; + style.top = ""; style.left = ""; style.position = ""; diff --git a/src/viewer.js b/src/viewer.js index 0c6f5969..51eaf7b3 100644 --- a/src/viewer.js +++ b/src/viewer.js @@ -509,6 +509,38 @@ $.extend( $.Viewer.prototype, $.EventHandler.prototype, $.ControlDock.prototype, return this; }, + + /** + * Function to destroy the viewer and clean up everything created by Open Seadragon + * @function + * @name OpenSeadragon.Viewer.prototype.destroy + */ + destroy: function( ) { + this.close(); + + this.removeAllHandlersForAllEvents(); + + // Go through top element (passed to us) and remove all children + // Use removeChild to make sure it handles SVG or any non-html + // also it performs better - http://jsperf.com/innerhtml-vs-removechild/15 + while (this.element.firstChild) { + this.element.removeChild(this.element.firstChild); + } + + // remove the mouse trackers - should we be cleaning up their callbacks? + delete this.keyboardCommandArea.innerTracker; + delete this.innerTracker; + delete this.outerTracker; + + // clear all our references to dom objects + this.canvas = null; + this.keyboardCommandArea = null; + this.container = null; + + // clear our reference to the main element - they will need to pass it in again, creating a new viewer + this.element = null; + }, + /** * @function From 68f9d675fc6aa34d44ac2431d2f8bdd352d8e9d7 Mon Sep 17 00:00:00 2001 From: Luke Murray Date: Thu, 8 Aug 2013 17:49:24 +1000 Subject: [PATCH 2/4] fix: calling viewer.destroy() multiple times throw an error. And a simple destroy to mousetracker --- src/eventhandler.js | 22 ++++++++-------------- src/mousetracker.js | 9 +++++++++ src/viewer.js | 26 ++++++++++++++++++-------- 3 files changed, 35 insertions(+), 22 deletions(-) diff --git a/src/eventhandler.js b/src/eventhandler.js index 430c23fc..b834b214 100644 --- a/src/eventhandler.js +++ b/src/eventhandler.js @@ -91,25 +91,19 @@ $.EventHandler.prototype = { /** - * Remove all event handler for a given event type. + * Remove all event handler for a given event type. If no type is given all event handlers for every event type is removed * @function * @param {String} eventName - Name of event for which all handlers are to be removed. */ removeAllHandlers: function( eventName ) { - this.events[ eventName ] = []; - }, - - - /** - * Remove every event handler for all event types - * @function - */ - removeAllHandlersForAllEvents: function( ) { - for (var eventType in this.events) { - this.events[eventType] = []; + if (eventName){ + this.events[ eventName ] = []; + } else{ + for (var eventType in this.events) { + this.events[eventType] = []; + } } - }, - + }, /** * Retrive the list of all handlers registered for a given event. diff --git a/src/mousetracker.js b/src/mousetracker.js index 4d81dc6b..1b4de92d 100644 --- a/src/mousetracker.js +++ b/src/mousetracker.js @@ -171,6 +171,15 @@ $.MouseTracker.prototype = { + /** + * Clean up any events or objects created the mouse tracker + * @function + */ + destroy: function() { + stopTracking( this ); + this.element = null; + }, + /** * Are we currently tracking events on this element. * @deprecated Just use this.tracking diff --git a/src/viewer.js b/src/viewer.js index 51eaf7b3..d3dd7579 100644 --- a/src/viewer.js +++ b/src/viewer.js @@ -499,7 +499,9 @@ $.extend( $.Viewer.prototype, $.EventHandler.prototype, $.ControlDock.prototype, this.viewport = this.preserveViewport ? this.viewport : null; //this.profiler = null; - this.canvas.innerHTML = ""; + if (this.canvas){ + this.canvas.innerHTML = ""; + } VIEWERS[ this.hash ] = null; delete VIEWERS[ this.hash ]; @@ -518,19 +520,27 @@ $.extend( $.Viewer.prototype, $.EventHandler.prototype, $.ControlDock.prototype, destroy: function( ) { this.close(); - this.removeAllHandlersForAllEvents(); + this.removeAllHandlers(); // Go through top element (passed to us) and remove all children // Use removeChild to make sure it handles SVG or any non-html // also it performs better - http://jsperf.com/innerhtml-vs-removechild/15 - while (this.element.firstChild) { - this.element.removeChild(this.element.firstChild); + if (this.element){ + while (this.element.firstChild) { + this.element.removeChild(this.element.firstChild); + } } - // remove the mouse trackers - should we be cleaning up their callbacks? - delete this.keyboardCommandArea.innerTracker; - delete this.innerTracker; - delete this.outerTracker; + // destroy the mouse trackers + if (this.keyboardCommandArea){ + this.keyboardCommandArea.innerTracker.destroy(); + } + if (this.innerTracker){ + this.innerTracker.destroy(); + } + if (this.outerTracker){ + this.outerTracker.destroy(); + } // clear all our references to dom objects this.canvas = null; From 0c662b8a8d1e414e0e44c5e0a48e9a04ed4123e6 Mon Sep 17 00:00:00 2001 From: Luke Murray Date: Mon, 12 Aug 2013 16:38:37 +1000 Subject: [PATCH 3/4] fix: comment updates and add a simple test for viewer.destroy --- src/eventhandler.js | 3 ++- src/mousetracker.js | 2 +- src/viewer.js | 3 ++- test/basic.js | 25 +++++++++++++++++++++++++ 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/src/eventhandler.js b/src/eventhandler.js index b834b214..f82a55b3 100644 --- a/src/eventhandler.js +++ b/src/eventhandler.js @@ -91,7 +91,8 @@ $.EventHandler.prototype = { /** - * Remove all event handler for a given event type. If no type is given all event handlers for every event type is removed + * Remove all event handlers for a given event type. If no type is given all + * event handlers for every event type is removed. * @function * @param {String} eventName - Name of event for which all handlers are to be removed. */ diff --git a/src/mousetracker.js b/src/mousetracker.js index 1b4de92d..dccea7c5 100644 --- a/src/mousetracker.js +++ b/src/mousetracker.js @@ -172,7 +172,7 @@ $.MouseTracker.prototype = { /** - * Clean up any events or objects created the mouse tracker + * Clean up any events or objects created by the mouse tracker. * @function */ destroy: function() { diff --git a/src/viewer.js b/src/viewer.js index d3dd7579..d9b02708 100644 --- a/src/viewer.js +++ b/src/viewer.js @@ -513,7 +513,8 @@ $.extend( $.Viewer.prototype, $.EventHandler.prototype, $.ControlDock.prototype, /** - * Function to destroy the viewer and clean up everything created by Open Seadragon + * Function to destroy the viewer and clean up everything created by + * OpenSeadragon. * @function * @name OpenSeadragon.Viewer.prototype.destroy */ diff --git a/test/basic.js b/test/basic.js index 61f36673..f83d7c80 100644 --- a/test/basic.js +++ b/test/basic.js @@ -208,4 +208,29 @@ viewer.open('/test/data/testpattern.dzi'); }); + // ---------- + asyncTest('Destroy', function() { + viewer.addHandler("open", function () { + start(); + // Check that the DOM has been modified + notEqual(0, $('#example').children().length); + + var closeHandler = function() { + viewer.removeHandler('close', closeHandler); + ok(true, 'Close event was sent on Destroy'); + }; + + viewer.addHandler('close', closeHandler); + viewer.destroy(); + + // Check that the DOM has been cleaned up + equal(0, $('#example').children().length); + equal(null, viewer.canvas); + equal(null, viewer.keyboardCommandArea); + equal(null, viewer.container); + equal(null, viewer.element); + }); + viewer.open('/test/data/testpattern.dzi'); + }); + })(); From ab82783cf1261cae6abec5220d45762517d2183e Mon Sep 17 00:00:00 2001 From: Luke Murray Date: Tue, 13 Aug 2013 10:33:12 +1000 Subject: [PATCH 4/4] update the comments and unit test --- src/eventhandler.js | 2 +- test/basic.js | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/src/eventhandler.js b/src/eventhandler.js index f82a55b3..8e608806 100644 --- a/src/eventhandler.js +++ b/src/eventhandler.js @@ -92,7 +92,7 @@ $.EventHandler.prototype = { /** * Remove all event handlers for a given event type. If no type is given all - * event handlers for every event type is removed. + * event handlers for every event type are removed. * @function * @param {String} eventName - Name of event for which all handlers are to be removed. */ diff --git a/test/basic.js b/test/basic.js index f83d7c80..7e0492c0 100644 --- a/test/basic.js +++ b/test/basic.js @@ -211,13 +211,13 @@ // ---------- asyncTest('Destroy', function() { viewer.addHandler("open", function () { - start(); // Check that the DOM has been modified notEqual(0, $('#example').children().length); + var closeCalled = false; var closeHandler = function() { viewer.removeHandler('close', closeHandler); - ok(true, 'Close event was sent on Destroy'); + closeCalled = true; }; viewer.addHandler('close', closeHandler); @@ -229,6 +229,8 @@ equal(null, viewer.keyboardCommandArea); equal(null, viewer.container); equal(null, viewer.element); + equal(true, closeCalled); + start(); }); viewer.open('/test/data/testpattern.dzi'); });