Add support for the file scheme to joinUrls. #5989
Conversation
|
@emackey, thanks for the pull request! Maintainers, we have a signed CLA from @emackey, so you can review this at any time.
I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome. 🌍 🌎 🌏 |
|
Travis failed... Was it something on this branch? How can I tell? |
|
Ah I guess Travis is OK then, since an edit to CHANGES.md fixed it. |
|
Thanks @emackey! This looks good to merge. Some tests are finicky, but you can restart the build on the log page if you think a failure was a fluke. |
Source/Core/joinUrls.js
Outdated
| @@ -78,6 +78,10 @@ define([ | |||
| if (baseUri.path !== '' && baseUri.path !== '/') { | |||
| url = url.replace(/\/?$/, '/'); | |||
| baseUri.path = baseUri.path.replace(/^\/?/g, ''); | |||
|
|
|||
| if (baseUri.authority === '') { | |||
There was a problem hiding this comment.
What exactly is this checking for? I'd like to better understand this change before we merge.
There was a problem hiding this comment.
On line 79, a regex is used to add a slash in case a slash is missing from the end of the authority. For example:
http://host => http://host/
http://host/ => http://host/
But with the file protocol, authority can be blank for local files. So:
file:// => file:// -- incorrect, should be file:///
In this case, the regex on line 79 does not add a needed slash after an empty authority, because it sees a slash that came before the authority as satisfying the expression.
There was a problem hiding this comment.
Originally, this test was if (url === 'file://'), but I think checking for the blank authority is potentially more generic.
There was a problem hiding this comment.
@mramato Are you OK with this now?
There was a problem hiding this comment.
Sorry, have been out on vacation. Can we just add a one line comment with your above explanation so future devs aren't confused. Once that's in, I'll merge.
|
Thanks @emackey ! |
Some environments allow local file access from JavaScript. In particular, Electron-based environments, such as the HTML preview window in Visual Studio Code, allow this.
For AnalyticalGraphicsInc/gltf-vscode#58.