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

Raise error event for network link failure #6158

Merged
merged 1 commit into from
Jan 30, 2018
Merged

Raise error event for network link failure #6158

merged 1 commit into from
Jan 30, 2018

Conversation

hpinkos
Copy link
Contributor

@hpinkos hpinkos commented Jan 25, 2018

Fixes #5921

We raise the error event in other places, but we missed this one.

@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 Jan 25, 2018

@thw0rted does this fix the problem you saw?

@thw0rted
Copy link
Contributor

LGTM

@juburr
Copy link
Contributor

juburr commented Jan 29, 2018

@hpinkos Will this silently fail and continue loading the rest of the static content like here? #5871
If not, this would be desirable.

I've seen this happen with other resources as well (for example, I believe this can also happen with external icon image hrefs when not using a proxy). Ideally it should continue loading the rest of the KML.

@hpinkos
Copy link
Contributor Author

hpinkos commented Jan 29, 2018

@jburr-nc this change doesn't change anything about the way KML currently loads. The only difference is that the error event will be raised in the event of a network link failure. You would only see different behavior in your app if you are listening to the error event.

I thought we fixed the problem where a datasource wouldn't load when an image for a billboard failed. I couldn't find the pull request related to that though. If you have an example KML where that happens, let me know and I can write up a new issue.

@hpinkos hpinkos requested a review from tfili January 30, 2018 17:52
@tfili tfili merged commit 0f9e501 into master Jan 30, 2018
@tfili tfili deleted the kml-network-error branch January 30, 2018 18:46
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