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

fix(fabric.Gradient): Guard against deep mutation on svg export for color exports #8196

Merged
merged 1 commit into from
Aug 26, 2022

Conversation

asturur
Copy link
Member

@asturur asturur commented Aug 26, 2022

This was an edge case, but since the fix is actually trivial and in a non hot code path, is still worth fixing.

@github-actions
Copy link
Contributor

github-actions bot commented Aug 26, 2022

Code Coverage Summary

> fabric@5.1.0 coverage:report
> nyc report --reporter=lcov --reporter=text

-----------|---------|----------|---------|---------|-----------------------------------------------
File       | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s                             
-----------|---------|----------|---------|---------|-----------------------------------------------
All files  |   82.05 |    74.69 |   84.44 |   80.67 |                                               
 fabric.js |   82.05 |    74.69 |   84.44 |   80.67 | ...-27584,27700-27701,27722-27763,27778-27937 
-----------|---------|----------|---------|---------|-----------------------------------------------

@asturur
Copy link
Member Author

asturur commented Aug 26, 2022

The bug was added in the migration, but a test was missing anyway.
The reason for the bug was the removal of a deep clone statement.

@asturur asturur merged commit af947d6 into master Aug 26, 2022
@ShaMan123
Copy link
Contributor

Was there a deep clone statement? I think this bug existed regardless

@asturur
Copy link
Member Author

asturur commented Aug 27, 2022

https://github.com/fabricjs/fabric.js/pull/8001/files#diff-2e1e3a2e5dc880cba81870b75734070b74d9282630f4b85b3f923999045fc110L222

wasn't the migration indeed, was before, i compared with v5.x and i assumed all the chages were from the ts migration

@asturur asturur deleted the add-failing-uts-gradient branch September 11, 2022 23:03
frankrousseau pushed a commit to cgwire/fabric.js that referenced this pull request Jan 6, 2023
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