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

Fix allow local-running Cesium to load its resources for Chromium #6197

Merged
merged 1 commit into from
Feb 22, 2018

Conversation

hpinkos
Copy link
Contributor

@hpinkos hpinkos commented Feb 8, 2018

@cesium-concierge
Copy link

Signed CLA is on file.

@hpinkos, thanks for the pull request! Maintainers, we have a signed CLA from @hpinkos, so you can review this at any time.

⚠️ I noticed that CHANGES.md has not been updated. If this change updates the public API in any way, fixes a bug, or makes any non-trivial update, please add a bullet point to CHANGES.md and comment on this pull request so we know it was updated. For more info, see the Pull Request Guidelines.


I am a bot who helps you make Cesium awesome! Contributions to my configuration are welcome.

🌍 🌎 🌏

@hpinkos
Copy link
Contributor Author

hpinkos commented Feb 8, 2018

@thw0rted this should fix your issue, right?

Hope you get your CLA issues figured out. It would be great to have you as a contributor!

@thw0rted
Copy link
Contributor

thw0rted commented Feb 8, 2018

I have tested the change and it does resolve the issue for my specific use case (in Chrome/Chromium, with --allow-file-access-from-files switch). I'll address other local-load issues in other threads.

@thw0rted
Copy link
Contributor

thw0rted commented Feb 8, 2018

Trackback to #5830

@hpinkos
Copy link
Contributor Author

hpinkos commented Feb 21, 2018

Can someone review this? @emackey?

@emackey
Copy link
Contributor

emackey commented Feb 21, 2018

I've been meaning to test the new resource stuff in VSCode, haven't gotten there yet.

In the meantime, this change seems fine for Chromium. The same behavior is not present in Firefox (origin is null for local files) but presumably one wouldn't run Cesium in a stand-alone Firefox engine the way one does for Electron/Chromium.

@hpinkos If you like I can build Cesium locally with and without this change and test the behavior out inside a VSCode plugin. Or you could just merge this, it looks fine.

@thw0rted
Copy link
Contributor

Just as a data point, I think our customers are more likely to run our product offline in Firefox than Chrome, just based on how the software baseline is set up, but I wouldn't expect it to be *common. I'm also fairly certain that the above change wouldn't affect Firefox because I don't think it returns status=0 for local XHRs.

@hpinkos
Copy link
Contributor Author

hpinkos commented Feb 22, 2018

Thanks @emackey! I think I'm just going to merge this now

@hpinkos hpinkos merged commit d207b1a into master Feb 22, 2018
@hpinkos hpinkos deleted the fix-file-loading branch February 22, 2018 15:10
@hpinkos
Copy link
Contributor Author

hpinkos commented Feb 22, 2018

Thanks @thw0rted, this is merged and will be included in Cesium 1.43

@pjcozzi
Copy link
Contributor

pjcozzi commented Feb 22, 2018

Does this warrant an update to CHANGES.md?

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.

5 participants