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

Handle XML parsing error sensibly #835

Merged

Conversation

davemevans
Copy link
Contributor

While testing another feature I discovered that XML parsing errors are handled differently across platforms and not caught in a sensible way on any. This could lead to the player attempting to continue with an incomplete manifest (Chrome, Safari) or fail but not for the correct reason (Firefox, but only because its error response contains a Processing Instruction node which causes the json conversion to barf).

This PR reliably detects parsererrors and forces a sensible (undefined) response from parser which makes the player act as I believe was intended. IE stop attempting to play the malformed manifest.

Please note I have NOT tested this on IE11/Edge as I don't have access to those browsers right now. Could someone try this out and let me know?

@KozhinM
Copy link
Contributor

KozhinM commented Oct 8, 2015

@bbcrddave, everything seems to work fine in Edge, but IE11 throws syntax error at the following code:
// src/lib/xml2json.js
ns = parser.parseFromString('<', 'text/xml')
Looks like IE just throws an exception when it is unable to parse xml.
I have not got a solution for this right now, but I will let you know if I come up with an idea.

@davemevans
Copy link
Contributor Author

@KozhinM Thanks for looking! Surprised about IE11 as I was under the impression DOMParser was not supposed to throw any exceptions.

I've updated the solution with some extra logic for IE11. (Note that I tested it by isolating the functions and running it in Win7/IE11 rather than as part of dash.js)

@KozhinM
Copy link
Contributor

KozhinM commented Oct 8, 2015

@bbcrddave, thanks for a quick response. The fix works fine. I'll merge the PR right away.

KozhinM added a commit that referenced this pull request Oct 8, 2015
Handle XML parsing error sensibly
@KozhinM KozhinM merged commit 3a1a8da into Dash-Industry-Forum:development Oct 8, 2015
@davemevans davemevans deleted the DetectParserError branch October 8, 2015 09:27
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.

None yet

2 participants