-
-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Fix RE used to correct quotes used in URLs for IE. #1740
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
Conversation
The `svgToImg()` function tries to normalize the SVG for compatibility with IE. However, currently it use a regular expression that is too greedy when matching the attribute values that need quotes changed from single to double. The result is that invalid XML is generated and will cause IE11 to fail with a `XML 5607` error and `svgToImg()` will never return. Edge will not complain but will receive invalid XML, which causes problems when used. This change restricts matching to only data surrounded by single quotes. Seems like a jasmine test should be added for `svgToImg()` along the lines of `it('Returns valid XML',...)`
Thanks very much for the PR! That might be enough to fix #699 |
@bobisch I'm having problem replicating the problem.
|
@etpinard - sorry, I thought we had some relevant examples, but I guess none highlight this particular issue. Unfortunately, I don't think this will do too much to really fix #699 and related issues since those probably have more to do with the IE limitations caused by tainting svg canvas data. But, this will make it possible to use some work-around(s) such as sending the data to the back-end for rendering or using canvg. Anyway, to get the error you need to have something that produces at least two |
@bobisch thanks for the info! Bundling up your patch and using it in https://codepen.io/etpinard/pen/WjqNLE gives the correct in IE11: 🎉🎉🎉🎉🎉🎉🎉 About adding a new test case, I can't find a way to test this logic outside of IE. Mocking the test environment such that it executes this normally IE-only block in What we should really do is implement #431 - so I'd vote for not adding any tests here, merge this in and include it in Sounds good? |
That seems reasonable to me. Thanks! Wouldn't it still be reasonable to just add a test that checks to see if |
That's the problem. XML parsers are different from browser to browser. A string that might be considered valid in one browser won't necessary be valid in another. |
The
svgToImg()
function tries to normalize the SVG for compatibilitywith IE. However, currently it use a regular expression that is too
greedy when matching the attribute values that need quotes changed from
single to double. The result is that invalid XML is generated and will
cause IE11 to fail with a
XML 5607
error andsvgToImg()
will neverreturn. Edge will not complain but will receive invalid XML, which
causes problems when used.
This change restricts matching to only data surrounded by single quotes.
Seems like a jasmine test should be added for
svgToImg()
along thelines of
it('Returns valid XML',...)
Also addresses: https://community.plot.ly/t/plotly-toimage-xml5607-error-in-ie11/4223