Fix undefined sheet of SVGStyleElement in Safari#159
Fix undefined sheet of SVGStyleElement in Safari#159knuhol merged 3 commits intocburgmer:masterfrom knuhol:master
Conversation
| var selectorRegex = matchingSimpleSelectorsRegex(simpleSelectorList); | ||
|
|
||
| asArray(element.querySelectorAll('style')).forEach(function (styleElement) { | ||
| if (typeof styleElement.sheet === 'undefined') { |
There was a problem hiding this comment.
I've taken up the habit of commenting on the workarounds that go into the library. It hopefully reduces the amount of "WTF" folks will have when reading this later. Especially considering that this library feels like a one big workaround.
Feel free to do the same, possibly linking to our discussion on the issue tracker.
There was a problem hiding this comment.
Yes it is a good idea, I did it.
|
I forgot. One thing that helps me (and hopefully others) maintain this project, is making sure all the pieces that make it into the code base have at least a test that documents why this is there. For the case we are discussing here, having one test, say, in https://github.com/akarienta/rasterizeHTML.js/blob/5c1be68cc899ea9aebc54ed46165650b8e95a648/test/specs/DocumentUtilSpec.js#L13 to make sure You can run the test in Safari by opening the test/SpecRunner.html. Apologies for the number of failing tests, I started developing a new feature a while ago that hasn't been wrapped up yet. I'll try to clean this up. |
|
I've read a nice post about opening up access for contributors early on. So you should be getting a notification on getting access. Feel free to merge this PR yourself. I'll try to get a release going afterwards. |
|
Thanks to the added spec I've discovered a problem with my fix, it should be all right now. I think it can be merged :) Thanks for the rights! |
2 participants
Fix of issue #158