-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Use JavaScript to redirect users #4642
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
notebook/templates/browser-open.html
Outdated
<head> | ||
<meta charset="UTF-8"> | ||
<meta http-equiv="refresh" content="1;url={{ open_url }}" /> | ||
<meta http-equiv="refresh" content="2;url={{ open_url }}" /> |
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 increased this to two seconds to let the JS version do its job first. Not sure what is best out of: disable the meta-refresh (if the JS redirect will work), reduce the time of the JS based refresh to have it take precedence or increase this constant to two from 1.
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 don't quite understand the reason to delay this. If it works, is there a reason to wait for js? Shouldn't this be preferred to js unless it's being ignored for some reason?
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.
For me/on Firefox the existing redirect mechanism doesn't work properly, but the JS based one does.
My thinking was that people who use the notebook have JS turned on so using JS should catch most people, then we try and get some more users by using the meta-refresh. I assumed that there was no reason to prefer meta-refresh over JS (maybe that isn't right) but the fact that meta-refresh doesn't work properly in FF is a reason to prefer JS over meta-refresh.
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 does "not work properly" mean? Does it redirect wrong, or just do nothing? I was assuming that the failure mode was that nothing happened and the js redirect would take over. Does having this on a shorter time mean that the js redirect doesn't work?
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.
You do get redirected but the page you end up with is:
The path shown in the location bar at the top is file:///Users/timsusername/Library/Jupyter/runtime/nbserver-86595-open.html
instead of http://localhost:8888
I assumed there would be a race between the two refresh methods if set to the same timeout. However when actually trying it locally about ten times I always got the JS redirect. So I changed it back to a 1s delay for the meta-refres.
cc @minrk @takluyver |
This pull request has been mentioned on Jupyter Community Forum. There might be relevant details there: |
Great, thanks! |
This is a followup to #4260.
This uses JS to do the redirection which works on Firefox (tested on FF 67.0b17 (64-bit)). The meta-refresh is still there so users without JS will be redirected as well.