Fix for when toObjects is called on a canvas with a clipPath #5556
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
asturur
reviewed
Mar 8, 2019
src/static_canvas.class.js
Outdated
| @@ -1177,7 +1177,7 @@ | |||
| objects: this._toObjects(methodName, propertiesToInclude), | |||
| }; | |||
| if (clipPath) { | |||
| data.clipPath = this._toObjectMethod(clipPath, methodName, propertiesToInclude); | |||
| data.clipPath = this.clipPath; | |||
There was a problem hiding this comment.
wait, this should call _toOject anyway no?
In this way we are exporting the instance itself.
We want it's object representation.
…sentation of clipPath object, without the klass prototype
… fix a seemingly unknown issue at the time attributed to the QUnit, but was in fact to do with the clipPath not having _toObject applied to it, thus it keeping the klass prototype, and not a plain JS object
asturur
reviewed
Mar 10, 2019
…the toObject method of the clipPath
|
@asturur changes to the test have been made now |
|
thanks! |
|
Hey guys, I just ran a quick test of this fix. While this does seem to resolve the issue with serializing a canvas with a clipPath, there appears to be a new bug when running Is this worth opening up a new issue?
|
|
i think there is a pr open for the load fix |
|
ah thanks, I missed that - #5641 |
Merged
thiagocunha
pushed a commit
to thiagocunha/fabric.js
that referenced
this pull request
Nov 18, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Details
Regarding issue: #5549
Bugs
_toObjectMethodis calling itself recursively if a clipPath is present on the root canvas.klassobject itself, instead of the expected argumentStringmethodName, which breaks the subsequent totoObjectmethod. The reason it breaks is because it's trying to find a method withmethodName, but because it's anobjectit fails and throws an error.Fix
_toObjectMethodand instead apply the clipPath to the scoped variabledataifclipPathexists, and returns that instead. This results in the expected behaviour when applying a clipPath with no errors.