Skip to content

Commit

Permalink
js: Don't trigger beforerender or rendered for discarded content
Browse files Browse the repository at this point in the history
A listener for `rendered` should not need to check if the content
really changed. Though, if such a listener is not triggered anymore,
`beforerender` listeners also must not be triggered, as they might
assume that the content is really being updated and their accompanied
`rendered` listener is triggered. (e.g. input-enrichment.js)

This might be breaking change and any `Behavior.renderHook` implementation
needs to be checked against it. Potentially also in third party modules.
As if such an implementation updates the container on its own,
`beforerender` listeners only have access to the updated container after
this change, while they had access to the original beforehand.

`rendered` listeners should not be that much affected, as for them the
change results in the same behavior as if no update has ever been
scheduled for the container.

fixes #5056
  • Loading branch information
nilmerg committed Jul 7, 2023
1 parent d311005 commit ef4bd56
Showing 1 changed file with 17 additions and 7 deletions.
24 changes: 17 additions & 7 deletions public/js/icinga/loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -873,7 +873,7 @@
}
})
} else {
this.renderContentToContainer(
let rendered = this.renderContentToContainer(
req.responseText,
req.$target,
req.action,
Expand All @@ -882,6 +882,10 @@
req.autosubmit || autoSubmit,
req.scripted
);

if (! rendered) {
req.discarded = true;
}
}

if (oldNotifications) {
Expand All @@ -905,6 +909,14 @@
req = reqOrError;
}

req.$target.data('lastUpdate', (new Date()).getTime());
delete this.requests[req.$target.attr('id')];
this.icinga.ui.fadeNotificationsAway();

if (req.discarded) {
return;
}

// Remove 'impact' class if there was such
if (req.$target.hasClass('impact')) {
req.$target.removeClass('impact');
Expand Down Expand Up @@ -957,10 +969,6 @@
}
}

req.$target.data('lastUpdate', (new Date()).getTime());
delete this.requests[req.$target.attr('id')];
this.icinga.ui.fadeNotificationsAway();

var extraUpdates = req.getResponseHeader('X-Icinga-Extra-Updates');
if (!! extraUpdates && req.getResponseHeader('X-Icinga-Redirect-Http') !== 'yes') {
$.each(extraUpdates.split(','), function (idx, el) {
Expand Down Expand Up @@ -1253,8 +1261,6 @@
}
}

$container.trigger('beforerender', [content, action, autorefresh, scripted, autoSubmit]);

var discard = false;
$.each(_this.icinga.behaviors, function(name, behavior) {
if (behavior.renderHook) {
Expand All @@ -1272,6 +1278,8 @@
});

if (! discard) {
$container.trigger('beforerender', [content, action, autorefresh, scripted, autoSubmit]);

if ($container.closest('.dashboard').length) {
var title = $('h1', $container).first().detach();
$container.html(title).append(content);
Expand Down Expand Up @@ -1337,6 +1345,8 @@

// Re-enable all click events (disabled as of performance reasons)
// $('*').off('click');

return ! discard;
},

/**
Expand Down

0 comments on commit ef4bd56

Please sign in to comment.