From 34988c7c4fd7adeb7688eb89829f1469572fb702 Mon Sep 17 00:00:00 2001 From: Jack Lukic Date: Tue, 17 May 2016 18:28:46 -0700 Subject: [PATCH] Fix possible memory leak in popup/sticky/visibility --- RELEASE-NOTES.md | 1 + src/definitions/behaviors/visibility.js | 35 ++++++++++++++++++++----- src/definitions/modules/popup.js | 6 +++-- src/definitions/modules/sticky.js | 35 ++++++++++++++++++++----- 4 files changed, 63 insertions(+), 14 deletions(-) diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md index 3cc0cfa80..792e57490 100644 --- a/RELEASE-NOTES.md +++ b/RELEASE-NOTES.md @@ -41,6 +41,7 @@ - **Search** - A previous unfinished XHR query aborting would cause the next query to fail #2779 - **Search** - Fixed an issue where `onResult` returning `false` would not prevent the search menu from hiding. Clicking on an empty results message will also no longer close the search results. #3856 #3870 - **Video** - Fixed issue with `change` behavior not working properly when correctly to change videos. +- **Sticky/Visibility** - Added mutation observer to teardown element with `destroy` if removed from DOM context, fixing a possible memory leak **Bugs** - **API** - Using `onResponse` with `dataType` other than JSON or JSONP would cause an error. (Not allowing plain text responses to be translated) #3653 diff --git a/src/definitions/behaviors/visibility.js b/src/definitions/behaviors/visibility.js index 4762c4c7e..9f312ed6e 100644 --- a/src/definitions/behaviors/visibility.js +++ b/src/definitions/behaviors/visibility.js @@ -68,6 +68,7 @@ $.fn.visibility = function(parameters) { element = this, disabled = false, + contextObserver, observer, module ; @@ -118,11 +119,15 @@ $.fn.visibility = function(parameters) { if(observer) { observer.disconnect(); } + if(contextObserver) { + contextObserver.disconnect(); + } $window .off('load' + eventNamespace, module.event.load) .off('resize' + eventNamespace, module.event.resize) ; $context + .off('scroll' + eventNamespace, module.event.scroll) .off('scrollchange' + eventNamespace, module.event.scrollchange) ; if(settings.type == 'fixed') { @@ -137,12 +142,11 @@ $.fn.visibility = function(parameters) { observeChanges: function() { if('MutationObserver' in window) { - observer = new MutationObserver(function(mutations) { - module.verbose('DOM tree modified, updating visibility calculations'); - module.timer = setTimeout(function() { - module.verbose('DOM tree modified, updating sticky menu'); - module.refresh(); - }, 100); + contextObserver = new MutationObserver(module.event.contextChanged); + observer = new MutationObserver(module.event.changed); + contextObserver.observe($context[0], { + childList : true, + subtree : true }); observer.observe(element, { childList : true, @@ -173,6 +177,25 @@ $.fn.visibility = function(parameters) { }, event: { + changed: function(mutations) { + module.verbose('DOM tree modified, updating visibility calculations'); + module.timer = setTimeout(function() { + module.verbose('DOM tree modified, updating sticky menu'); + module.refresh(); + }, 100); + }, + contextChanged: function(mutations) { + [].forEach.call(mutations, function(mutation) { + if(mutation.removedNodes) { + [].forEach.call(mutation.removedNodes, function(node) { + if(node == element || $(node).find(element).length > 0) { + module.debug('Element removed from DOM, tearing down events'); + module.destroy(); + } + }); + } + }); + }, resize: function() { module.debug('Window resized'); if(settings.refreshOnResize) { diff --git a/src/definitions/modules/popup.js b/src/definitions/modules/popup.js index d19e2aef7..a3139ce1c 100644 --- a/src/definitions/modules/popup.js +++ b/src/definitions/modules/popup.js @@ -112,7 +112,6 @@ $.fn.popup = function(parameters) { } }, - refresh: function() { if(settings.popup) { $popup = $(settings.popup).eq(0); @@ -159,6 +158,9 @@ $.fn.popup = function(parameters) { destroy: function() { module.debug('Destroying previous module'); + if(contextObserver) { + contextObserver.disconnect(); + } // remove element only if was created dynamically if($popup && !settings.preserve) { module.removePopup(); @@ -204,7 +206,7 @@ $.fn.popup = function(parameters) { module.set.position(); } }, - contextMutation: function(mutations) { + contextChanged: function(mutations) { [].forEach.call(mutations, function(mutation) { if(mutation.removedNodes) { [].forEach.call(mutation.removedNodes, function(node) { diff --git a/src/definitions/modules/sticky.js b/src/definitions/modules/sticky.js index 8a423ec68..a764299f9 100644 --- a/src/definitions/modules/sticky.js +++ b/src/definitions/modules/sticky.js @@ -64,6 +64,8 @@ $.fn.sticky = function(parameters) { || function(callback) { setTimeout(callback, 0); }, element = this, + + contextObserver, observer, module ; @@ -97,6 +99,9 @@ $.fn.sticky = function(parameters) { destroy: function() { module.verbose('Destroying previous instance'); module.reset(); + if(contextObserver) { + contextObserver.disconnect(); + } if(observer) { observer.disconnect(); } @@ -115,12 +120,11 @@ $.fn.sticky = function(parameters) { context = $context[0] ; if('MutationObserver' in window) { - observer = new MutationObserver(function(mutations) { - clearTimeout(module.timer); - module.timer = setTimeout(function() { - module.verbose('DOM tree modified, updating sticky menu', mutations); - module.refresh(); - }, 100); + contextObserver = new MutationObserver(module.event.contextChanged); + observer = new MutationObserver(module.event.changed); + contextObserver.observe($context[0], { + childList : true, + subtree : true }); observer.observe(element, { childList : true, @@ -178,6 +182,25 @@ $.fn.sticky = function(parameters) { }, event: { + changed: function(mutations) { + clearTimeout(module.timer); + module.timer = setTimeout(function() { + module.verbose('DOM tree modified, updating sticky menu', mutations); + module.refresh(); + }, 100); + }, + contextChanged: function(mutations) { + [].forEach.call(mutations, function(mutation) { + if(mutation.removedNodes) { + [].forEach.call(mutation.removedNodes, function(node) { + if(node == element || $(node).find(element).length > 0) { + module.debug('Element removed from DOM, tearing down events'); + module.destroy(); + } + }); + } + }); + }, load: function() { module.verbose('Page contents finished loading'); requestAnimationFrame(module.refresh);