theDialogManager addHandler no arguments passed to handler function

[nelu]

Please complete the following tasks.

• Web browser cache cleared
• Link provided to install script if applicable
• Not using broken rtinst install script
• Web browser, ruTorrent, PHP and OS version provided

Tell us about your environment

Docker

Tell us how you installed ruTorrent

Docker

Describe the bug

The handler and the previous queued function are being called with no arguments in addHandler method:

ruTorrent/js/objects.js

Line 251 in 25679a4

handler();

	addHandler: function(id, type, handler) {
		if ($type(this.items[id])) {
			const existing = this.items[id][type];
			if (existing) {
				this.items[id][type] = function() {
					existing();
					handler();
				};
			} else {
				this.items[id][type] = handler;
			}
		}
		return this;
	},

The proper function handler signature is this (when called):

ruTorrent/js/objects.js

Line 236 in 25679a4

this.items[id].afterHide(id);

			this.items[id].afterHide(id);

Steps to reproduce

Check if handler function has any arg value when multiple handler are registered for the same event.

Expected behavior

Use something like this

				this.items[id][type] = function(id) {
					existing(id);
					handler(id);
				};

Additional context

Noticed while working with the _getDir plugin on the hide events

[jevenski]

This is not a very efficient nor maintainable solution to manage dialog open/close event handlers in today's perspective. I'm thinking about defining custom events for dialog opening and closing events, and register/deregister event handlers for them. This was inspired by Bootstrap's show, shown, hide, and hidden events. We could have migrated to Bootstrap's Modal component for easier management, but it has a limitation that only one dialog window is allowed to open at the same time. So we need to stick with our own dialog system and adopt those custom events.

[nelu]

You could use the standard event system available: element.on / trigger

       // deregistering when cleaning up
        var unbind = function (e, task) {
            if (task.no === runTask.no) {
                def.resolve(task);
            }
        };

        $(document).on(flm.EVENTS.taskDone, unbind);

        def.promise().then(function (task) {
            $(document).off(flm.EVENTS.taskDone, unbind);
            return task;
        });
....
        $(document).trigger(flm.EVENTS.taskDone, task);

Which would make sense since this ships natively with the browser/jq. see https://api.jquery.com/category/events/event-handler-attachment/ and https://developer.mozilla.org/en-US/docs/Web/Events/Creating_and_triggering_events
Also uing bootstraps dialog system ON TOP of the current system seems quite doable (except bootstrap js part).

[jevenski]

Yep, that's what I'm talking about. I just haven't got time to bring that thoight into reality yet. Still struggling with cleaning up code of the s-table class.

[nelu]

Alrighty then, also these 'nice-to-have' events for reference:

    // ruTorrent component overrides
    plugin.flmConfig = theWebUI.config;
    theWebUI.config = (data) => $(document).trigger('theWebUI:config', [data]) && plugin.flmConfig.call(theWebUI, data);
    $(document).on('theWebUI:config', () => plugin.canChangeTabs() && plugin.ui.setConfig());

    plugin.flmOnShow = theTabs.onShow;
    theTabs.onShow = (id) => $(document).trigger('theTabs:onShow', [id]) && plugin.flmOnShow.call(theTabs, id);

    plugin.flmTabsShow = theTabs.show;
    theTabs.show = (id) => {
        $(document).trigger('theTabs:show', [id]);
        plugin.flmTabsShow.call(theTabs, id);
    };

    plugin.addAndShowSettings = theWebUI.addAndShowSettings;
    theWebUI.addAndShowSettings = (data) => $(document).trigger('theWebUI:addAndShowSettings', [data]) &&
        plugin.addAndShowSettings.call(theWebUI, data);

    plugin.flmSetSettings = theWebUI.setSettings;
    theWebUI.setSettings = (data) => $(document).trigger('theWebUI:setSettings', [data]) &&
        plugin.flmSetSettings.call(theWebUI, data);

    plugin.createTorrentFileMenu = theWebUI.createFileMenu;
    theWebUI.createFileMenu = (e, data) => (plugin.createTorrentFileMenu.call(theWebUI, e, data) &&
        $(document).trigger('theWebUI:createFileMenu', [data, e]));