-
-
Notifications
You must be signed in to change notification settings - Fork 8.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[JENKINS-73592] refresh buildhistory widget when tab was hidden before #9605
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
/label ready-for-merge
This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.
Thanks!
@@ -30,6 +30,9 @@ function load(options = {}) { | |||
|
|||
// Avoid fetching if the page isn't active | |||
if (document.hidden) { | |||
if (buildHistoryPage.dataset.pageHasUp === "false") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What the relevance of this condition?
It seems that the second problem (switching tabs) would still remain if I page through the build history first?
Is this just making this change consistent with a pre-existing bug? In testing it seems I'm not getting AJAX refreshes on page 2 of the widget.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed when you're not on the first page of the build history and there is a running build, this will never get updated (independent of this change).
But that behaviour can also be seen on 2.462.x and earlier. So this is a long standing bug/limitation.
replaced by #9624 |
The current implementation of the
load
method exits early when the document is hidden to avoid unnecessary calls to the backend. But this avoids that the refreshTimeout is properly (re-)created.This leads to 2 effects:
See JENKINS-73592.
Testing done
Manual verified with the mentioned steps above that history widget is properly updated.
Proposed changelog entries
Proposed upgrade guidelines
N/A
Submitter checklist
Desired reviewers
@mention
Before the changes are marked as
ready-for-merge
:Maintainer checklist