-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Delay reloading the page if an ajax error occurs, show notification #24126
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
Conversation
| setTimeout(function() { | ||
| if (!self._userIsNavigatingAway && !self._reloadCalled) { | ||
| OC.reload(); | ||
| OC.Notification.show(t('core', 'Problem loading page, reloading in 5 seconds')); |
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.
Note that this can happen for timed out sessions too. Should the text say "Problem doing request" instead ?
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.
I think it's acceptable to see 'problem loading page' if the session times out, since it's some part of the page that has failed, not the specific request to load the HTML page. Also 'request' is far more technical than 'page'
|
Sounds good, better than the page going nuts. |
|
Makes sense 👍 |
|
jsunit seems unhappy |
|
@PVince81 Mind re-reviewing this along with the modified tests? |
|
👍 |
|
Let's backport this to 9.0 to avoid false positives to cause a sudden reload: #25035 |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
E.g. if there's an error loading some part of the admin settings page (just happened to me), the page will reload continuously with no clear indication that there's something wrong. This PR puts a 5 second delay on the reload, and prints a notification to make things clear. I would have liked to also print the URL that was failing, but the xhr object doesn't store the right information and getting the URL through to the error handler seems awkward.
cc @PVince81 @jancborchardt