From 06e5d07650b2ab729623411019ad0180e3bc9653 Mon Sep 17 00:00:00 2001 From: Chad Date: Mon, 8 Feb 2021 12:56:56 +1300 Subject: [PATCH] Fix eventService memory leaks (#9752) * fix leak by unbinding event from $rootscope * destroy more events * $scope unavailable * no need to destroy in services as they are singletons Co-authored-by: nzdev Co-authored-by: Nathan Woulfe --- .../content/umbvariantcontenteditors.directive.js | 4 ++-- .../components/editor/umbeditorheader.directive.js | 5 ++++- .../src/common/services/editor.service.js | 3 +-- .../src/common/services/localization.service.js | 1 + .../src/common/services/navigation.service.js | 1 + .../prevalueeditors/treesourcetypepicker.controller.js | 10 ++++++++-- .../colorpicker/multicolorpicker.controller.js | 10 +++++++--- .../nestedcontent/nestedcontent.controller.js | 6 ++++-- .../src/views/relationtypes/edit.controller.js | 5 ++++- 9 files changed, 32 insertions(+), 13 deletions(-) diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbvariantcontenteditors.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbvariantcontenteditors.directive.js index c3fd0dc9c4..3e227bfcb3 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbvariantcontenteditors.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/content/umbvariantcontenteditors.directive.js @@ -187,8 +187,7 @@ } } - eventsService.on("editors.content.splitViewRequest", (_, args) => requestSplitView(args)); - + var unbindSplitViewRequest = eventsService.on("editors.content.splitViewRequest", (_, args) => requestSplitView(args)); /** Closes the split view */ function closeSplitView(editorIndex) { // TODO: hacking animation states - these should hopefully be easier to do when we upgrade angular @@ -201,6 +200,7 @@ $location.search({"cculture": culture, "csegment": vm.editors[0].content.segment}); splitViewChanged(); + unbindSplitViewRequest(); } /** diff --git a/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorheader.directive.js b/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorheader.directive.js index 2dfb0d5158..6f272f1ea2 100644 --- a/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorheader.directive.js +++ b/src/Umbraco.Web.UI.Client/src/common/directives/components/editor/umbeditorheader.directive.js @@ -357,9 +357,12 @@ Use this directive to construct a header inside the main editor window. scope.$emit("$changeTitle", title); } - $rootScope.$on('$setAccessibleHeader', function (event, isNew, editorFor, nameLocked, name, contentTypeName, setTitle) { + var unbindEventHandler = $rootScope.$on('$setAccessibleHeader', function (event, isNew, editorFor, nameLocked, name, contentTypeName, setTitle) { setAccessibilityHeaderDirective(isNew, editorFor, nameLocked, name, contentTypeName, setTitle); }); + scope.$on('$destroy', function () { + unbindEventHandler(); + }); } diff --git a/src/Umbraco.Web.UI.Client/src/common/services/editor.service.js b/src/Umbraco.Web.UI.Client/src/common/services/editor.service.js index e512e52643..08cd67460e 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/editor.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/editor.service.js @@ -179,8 +179,7 @@ When building a custom infinite editor view you can use the same components as a } else { focus(); } - }); - + }); /** * @ngdoc method diff --git a/src/Umbraco.Web.UI.Client/src/common/services/localization.service.js b/src/Umbraco.Web.UI.Client/src/common/services/localization.service.js index f8493ab39d..99162eaf53 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/localization.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/localization.service.js @@ -330,6 +330,7 @@ angular.module('umbraco.services') resourceFileLoadStatus = "none"; resourceLoadingPromise = []; }); + // return the local instance when called return service; diff --git a/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js b/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js index 3164c3ab19..c628e3a5b1 100644 --- a/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js +++ b/src/Umbraco.Web.UI.Client/src/common/services/navigation.service.js @@ -30,6 +30,7 @@ function navigationService($routeParams, $location, $q, $injector, eventsService var element = $(args.element); element.addClass('above-backdrop'); }); + //A list of query strings defined that when changed will not cause a reload of the route var nonRoutingQueryStrings = ["mculture", "cculture", "csegment", "lq", "sr"]; diff --git a/src/Umbraco.Web.UI.Client/src/views/prevalueeditors/treesourcetypepicker.controller.js b/src/Umbraco.Web.UI.Client/src/views/prevalueeditors/treesourcetypepicker.controller.js index a87377c84b..dcc9add395 100644 --- a/src/Umbraco.Web.UI.Client/src/views/prevalueeditors/treesourcetypepicker.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/prevalueeditors/treesourcetypepicker.controller.js @@ -83,8 +83,8 @@ function TreeSourceTypePickerController($scope, contentTypeResource, mediaTypeRe $scope.model.value = _.pluck(vm.itemTypes, "alias").join(); angularHelper.getCurrentForm($scope).$setDirty(); } - - eventsService.on("treeSourceChanged", function (e, args) { + var evts = []; + evts.push(eventsService.on("treeSourceChanged", function (e, args) { // reset the model value if we changed node type (but not on the initial load) if (!!currentItemType && currentItemType !== args.value) { vm.itemTypes = []; @@ -92,6 +92,12 @@ function TreeSourceTypePickerController($scope, contentTypeResource, mediaTypeRe } currentItemType = args.value; init(); + })); + + $scope.$on('$destroy', function () { + for (var e in evts) { + eventsService.unsubscribe(evts[e]); + } }); } diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/multicolorpicker.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/multicolorpicker.controller.js index 6312ab9df6..967137b930 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/multicolorpicker.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/colorpicker/multicolorpicker.controller.js @@ -48,11 +48,15 @@ } }); } - - eventsService.on("toggleValue", function (e, args) { + var evts = []; + evts.push(eventsService.on("toggleValue", function (e, args) { vm.labelEnabled = args.value; + })); + $scope.$on('$destroy', function () { + for (var e in evts) { + eventsService.unsubscribe(evts[e]); + } }); - if (!Utilities.isArray($scope.model.value)) { //make an array from the dictionary var items = []; diff --git a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.controller.js b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.controller.js index a629bd2eea..f262647a0d 100644 --- a/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/propertyeditors/nestedcontent/nestedcontent.controller.js @@ -507,8 +507,10 @@ vm.showPaste = clipboardService.hasEntriesOfType(clipboardService.TYPES.ELEMENT_TYPE, contentTypeAliases); } - eventsService.on("clipboardService.storageUpdate", checkAbilityToPasteContent); - + var storageUpdate = eventsService.on("clipboardService.storageUpdate", checkAbilityToPasteContent); + $scope.$on('$destroy', function () { + storageUpdate(); + }); var notSupported = [ "Umbraco.Tags", "Umbraco.UploadField", diff --git a/src/Umbraco.Web.UI.Client/src/views/relationtypes/edit.controller.js b/src/Umbraco.Web.UI.Client/src/views/relationtypes/edit.controller.js index a0b2a6afa1..62117728d0 100644 --- a/src/Umbraco.Web.UI.Client/src/views/relationtypes/edit.controller.js +++ b/src/Umbraco.Web.UI.Client/src/views/relationtypes/edit.controller.js @@ -54,11 +54,14 @@ function RelationTypeEditController($scope, $routeParams, relationTypeResource, }); // load references when the 'relations' tab is first activated/switched to - eventsService.on("app.tabChange", function (event, args) { + var appTabChange = eventsService.on("app.tabChange", function (event, args) { if (args.alias === "relations") { loadRelations(); } }); + $scope.$on('$destroy', function () { + appTabChange(); + }); // Inital page/overview API call of relation type relationTypeResource.getById($routeParams.id)