Allow for hidden port 80 #128
Conversation
ghost
mentioned this pull request
| @@ -1,6 +1,10 @@ | |||
| (function refresh () { | |||
| var verboseLogging = false | |||
| var socketUrl = window.location.origin.replace() // This is dynamically populated by the reload.js file before it is sent to the browser | |||
| var socketUrl = window.location.origin | |||
| if (!window.location.origin.match(/:[0-9]+/)) { | |||
There was a problem hiding this comment.
We probably want account for port 443 too I'm thinking?
And I'd like this all in a series of if statements.
I'm thinking
if (!window.location.origin.match(/:[0-9]+/)) {
}
else if (https regex for port 443) {
}
else {
socketUrl = socketUrl.replace() // This is dynamically populated by the reload.js file before it is sent to the browser
}What are your thoughts on this?
There was a problem hiding this comment.
Why does it need to change for port 443?
There was a problem hiding this comment.
Because 443 is also an anonymous port when running https. Might not be relevant though for the command line program?
There was a problem hiding this comment.
Seems like a good idea. I'll look into it.
There was a problem hiding this comment.
I've made a pull request #129 that does this and has been tested.
There was a problem hiding this comment.
I think this block of code would be safe if it was like:
if (!window.location.origin.match(/:[0-9]+/)) {
socketUrl = window.location.origin + ':80'
}
else {
socketUrl = socketUrl.replace() // This is dynamically populated by the reload.js file before it is sent to the browser
}
|
Other than above comment is working great |
ghost
mentioned this pull request
|
This looks great, thank you for the quick reporting and subsequent PR!! |
Should fix #126. @Yamboy1 had the correct regex to append 80 if the browser didn't supply it, but
reload.jsalso needed to be updated to correctly replace the new structure.