From 86e901f375280f4e59a52690f5b5b5473fb4898f Mon Sep 17 00:00:00 2001 From: Jermolene Date: Wed, 6 May 2015 08:07:12 +0100 Subject: [PATCH] Fix event handler leak for modals and notifications Also add support for passing custom variables into notifications. Fixes #1694 --- core/modules/utils/dom/modal.js | 17 +++++++++-------- core/modules/utils/dom/notifier.js | 17 ++++++++++++----- 2 files changed, 21 insertions(+), 13 deletions(-) diff --git a/core/modules/utils/dom/modal.js b/core/modules/utils/dom/modal.js index f753622c1..2a966a7db 100644 --- a/core/modules/utils/dom/modal.js +++ b/core/modules/utils/dom/modal.js @@ -29,6 +29,7 @@ Options include: Modal.prototype.display = function(title,options) { options = options || {}; var self = this, + refreshHandler, duration = $tw.utils.getAnimationDuration(), tiddler = this.wiki.getTiddler(title); // Don't do anything if the tiddler doesn't exist @@ -83,9 +84,6 @@ Modal.prototype.display = function(title,options) { variables: variables }); headerWidgetNode.render(headerTitle,null); - this.wiki.addEventListener("change",function(changes) { - headerWidgetNode.refresh(changes,modalHeader,null); - }); // Render the body of the message var bodyWidgetNode = this.wiki.makeTranscludeWidget(title,{ parentWidget: $tw.rootWidget, @@ -93,9 +91,6 @@ Modal.prototype.display = function(title,options) { variables: variables }); bodyWidgetNode.render(modalBody,null); - this.wiki.addEventListener("change",function(changes) { - bodyWidgetNode.refresh(changes,modalBody,null); - }); // Setup the link if present if(options.downloadLink) { modalLink.href = options.downloadLink; @@ -135,11 +130,17 @@ Modal.prototype.display = function(title,options) { variables: variables }); footerWidgetNode.render(modalFooterButtons,null); - this.wiki.addEventListener("change",function(changes) { + // Set up the refresh handler + refreshHandler = function(changes) { + headerWidgetNode.refresh(changes,modalHeader,null); + bodyWidgetNode.refresh(changes,modalBody,null); footerWidgetNode.refresh(changes,modalFooterButtons,null); - }); + }; + this.wiki.addEventListener("change",refreshHandler); // Add the close event handler var closeHandler = function(event) { + // Remove our refresh handler + self.wiki.removeEventListener("change",refreshHandler); // Decrease the modal count and adjust the body class self.modalCount--; self.adjustPageClass(); diff --git a/core/modules/utils/dom/notifier.js b/core/modules/utils/dom/notifier.js index 31c5c8c92..ca8e54ad7 100644 --- a/core/modules/utils/dom/notifier.js +++ b/core/modules/utils/dom/notifier.js @@ -27,21 +27,26 @@ Options include: Notifier.prototype.display = function(title,options) { options = options || {}; // Create the wrapper divs - var notification = document.createElement("div"), + var self = this, + notification = document.createElement("div"), tiddler = this.wiki.getTiddler(title), - duration = $tw.utils.getAnimationDuration(); + duration = $tw.utils.getAnimationDuration(), + refreshHandler; // Don't do anything if the tiddler doesn't exist if(!tiddler) { return; } // Add classes $tw.utils.addClass(notification,"tc-notification"); + // Create the variables + var variables = $tw.utils.extend({currentTiddler: title},options.variables); // Render the body of the notification - var widgetNode = this.wiki.makeTranscludeWidget(title,{parentWidget: $tw.rootWidget, document: document}); + var widgetNode = this.wiki.makeTranscludeWidget(title,{parentWidget: $tw.rootWidget, document: document, variables: variables}); widgetNode.render(notification,null); - this.wiki.addEventListener("change",function(changes) { + refreshHandler = function(changes) { widgetNode.refresh(changes,notification,null); - }); + }; + this.wiki.addEventListener("change",refreshHandler); // Set the initial styles for the notification $tw.utils.setStyle(notification,[ {opacity: "0"}, @@ -60,6 +65,8 @@ Notifier.prototype.display = function(title,options) { ]); // Set a timer to remove the notification window.setTimeout(function() { + // Remove our change event handler + self.wiki.removeEventListener("change",refreshHandler); // Force layout and animate the notification away $tw.utils.forceLayout(notification); $tw.utils.setStyle(notification,[