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

Add support for empty <style/> tags #6169

Merged
merged 1 commit into from
Feb 28, 2020
Merged

Add support for empty <style/> tags #6169

merged 1 commit into from
Feb 28, 2020

Conversation

iherasymenko
Copy link
Contributor

No description provided.

@asturur
Copy link
Member

asturur commented Feb 23, 2020

Seems good to me.
Would you please remove the IE9 comment and the relative code? ie9 is no more supported.

@iherasymenko
Copy link
Contributor Author

Sure thing. Done!

@asturur
Copy link
Member

asturur commented Feb 23, 2020

sorry could i ask you a test for loadSVGFromString that will keep this feature working from regression? A unit test should be enough. sorry for not asking before

@iherasymenko
Copy link
Contributor Author

No problem at all. I've just pushed the changes. I added a unit test for the getCssRule method.

Please let me know if you indeed want me to a test case for loadSVGFromString.

Thanks!

@iherasymenko
Copy link
Contributor Author

Sorry, the test failing in Chrome and Firefox; works fine on Node though. I'd appreciate if you could tell me how to see the stacktrace of the failing test.

test/unit/parser.js Outdated Show resolved Hide resolved
@iherasymenko
Copy link
Contributor Author

@asturur done. The CI pipeline is now green :-)

@asturur
Copy link
Member

asturur commented Feb 28, 2020

perfect! less code and is functional.

@asturur asturur merged commit 52901c5 into fabricjs:master Feb 28, 2020
@iherasymenko
Copy link
Contributor Author

Thank you 😊

This pull request was closed.
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.

2 participants