Skip to content

Commit 32bd47c

Browse files
committed
Improve code robustness per code review feedback
- Add defensive event listener cleanup in dialog close callbacks - Use global cleanup function to prevent memory leaks - Only execute cleanup when navigation listeners exist - Use .closest('li') instead of .parent() for DOM traversal robustness - Makes code resilient to HTML structure changes in jqueryFileTree - Fixes ReferenceError when closing dialogs without navigation setup Addresses code review items: - Line 460-461: Event listener cleanup fragility - Line 637-660: DOM traversal robustness
1 parent 1dbd0f3 commit 32bd47c

1 file changed

Lines changed: 19 additions & 4 deletions

File tree

emhttp/plugins/dynamix/Browse.page

Lines changed: 19 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -404,6 +404,9 @@ setTimeout(function(){filemonitor.start();},3000);
404404
var FOLDER_EXPAND_DELAY = 300; // Delay per folder expansion in openFolderRecursive (ms) - allows DOM updates to complete
405405
var NAVIGATION_BUFFER = 500; // Additional buffer time for navigation completion (ms) - ensures all async operations finish
406406

407+
// Global cleanup function for dialog close - set by setupTargetNavigation() if navigation is used
408+
var dfm_cleanupNavigationListeners = null;
409+
407410
function setupTargetNavigation() {
408411
var $target = dfm.window.find('#dfm_target');
409412
if (!$target.length) return;
@@ -558,16 +561,20 @@ function setupTargetNavigation() {
558561
}
559562
});
560563

561-
// Cleanup on dialog close
562-
dfm.window.on('dialogclose', function() {
564+
// Set global cleanup function to remove event listeners on dialog close
565+
dfm_cleanupNavigationListeners = function() {
563566
document.removeEventListener('click', treeClickHandler, true);
564567
document.removeEventListener('click', popularClickHandler, true);
565568
if (inputElement) {
566569
inputElement.removeEventListener('mousedown', preventClose, true);
567570
inputElement.removeEventListener('focus', preventClose, true);
568571
inputElement.removeEventListener('click', preventClose, true);
569572
}
570-
});
573+
dfm_cleanupNavigationListeners = null; // Clear reference after cleanup
574+
};
575+
576+
// Also bind to dialogclose event as additional safety
577+
dfm.window.on('dialogclose', dfm_cleanupNavigationListeners);
571578
}
572579

573580
function closeFolderPath(path) {
@@ -652,7 +659,7 @@ function openFolderRecursive($tree, pickroot, parts, index, maxDepth) {
652659

653660
if ($folderLink.length === 0) return;
654661

655-
var $folderLi = $folderLink.parent();
662+
var $folderLi = $folderLink.closest('li');
656663

657664
if ($folderLi.hasClass('expanded')) {
658665
setTimeout(function() {
@@ -801,6 +808,10 @@ function doAction(action, title, id) {
801808
modal: true,
802809
close: function() {
803810
$('.ui-dfm').off('mousedown.dfmFileTree');
811+
// Cleanup navigation event listeners if navigation was set up
812+
if (typeof dfm_cleanupNavigationListeners === 'function') {
813+
dfm_cleanupNavigationListeners();
814+
}
804815
},
805816
open: function() {
806817
addActionWarning(action, false);
@@ -1078,6 +1089,10 @@ function doActions(action, title) {
10781089
modal: true,
10791090
close: function() {
10801091
$('.ui-dfm').off('mousedown.dfmFileTree');
1092+
// Cleanup navigation event listeners if navigation was set up
1093+
if (typeof dfm_cleanupNavigationListeners === 'function') {
1094+
dfm_cleanupNavigationListeners();
1095+
}
10811096
},
10821097
open: function() {
10831098
addActionWarning(action, true);

0 commit comments

Comments
 (0)