Skip to content
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

Bug/654 redirect to stats.php on disconnect #672

Merged
merged 3 commits into from
Mar 15, 2024

Conversation

JahleelAbraham
Copy link
Contributor

The issue outlined by #654 is caused when the client disconnects from the internet and can no longer send a request to /ajax/stats.php. The code that causes this is a commit made to solve issue #474 which sends the entire browser to the URL /ajax/stats.php/ because the background request failed.

The solution proposed by this PR works around both of these issues by create a way to create exception to the redirection when the request fails by simply passing false to the optional parameter redirectOnFail.

This is by no means a proper solutions and a better solution for both issue #654 and #474 might be needed to be discussed.

Copy link
Contributor

@andrey-utkin andrey-utkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add true/false for the new argument in all other call sites. Or is there only one?

Copy link
Contributor

@andrey-utkin andrey-utkin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Looks good to me.
I am going to run a test with these changes and merge if it's all good (which it should be, given the clarity of changes).
(With regard to git history, I am going to squash the second and third commits into the first.)

@andrey-utkin
Copy link
Contributor

Tested this manually, the fix holds up - leaving the web page to idle for a long time, and deliberately disconnecting machine (shortly, or until requests time out) doesn't cause any redirect. Merging.

@andrey-utkin andrey-utkin merged commit ee91de2 into master Mar 15, 2024
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Browser disconnection redirects the browser to /ajax/stats.php
2 participants