fix(): nested duplicated clipPath causes infinite recursion

[Smrtnyk]

fixes #10659

[codesandbox]

Review or Edit in CodeSandbox

Open the branch in Web Editor • VS Code • Insiders

Open Preview

[github-actions]

Build Stats

file / KB (diff)	bundled	minified
fabric	918.154 (+0.295)	302.264 (+0.098)

[Smrtnyk]

the unit test I added hangs on vitest without the fix

[asturur]

if we are using gradientDef.def, the if condition above needs to check for that.

[Smrtnyk]

good catch will add few more unit tests

[Smrtnyk]

I pushed new commit, please recheck

[asturur]

I m not sure how this fix works.
We need more svg examples to be sure this is a fix.
i didn't check why it was recursively looping on itself.
If it was for the clippath.clippath reason, i do not think this fix can work for real use case scenario outside this simeple one. We need to build a better use case to test and be sure legit use cases are rendered correctly.

I didn't dig in the code when i checked this issue, but my impression was that i would have had to duplicate the double referenced clipPath with a clone of itself in order to avoid the infinite recursion

[Smrtnyk]

I m not sure how this fix works. We need more svg examples to be sure this is a fix. i didn't check why it was recursively looping on itself. If it was for the clippath.clippath reason, i do not think this fix can work for real use case scenario outside this simeple one. We need to build a better use case to test and be sure legit use cases are rendered correctly.

I didn't dig in the code when i checked this issue, but my impression was that i would have had to duplicate the double referenced clipPath with a clone of itself in order to avoid the infinite recursion

you are right
I might be just plastering over a bigger issue
lets close for now and I can come back to it when I have more time, if it doesn't get fixed in the mean time

[asturur]

Leave it open, the hardest task is to make an svg that has a nested clipPath and that has visual differences with one that doesn't have a nested clipPath. Once we have a reproducible use case of an svg that can render something that works we can add it here and see what happens

[Smrtnyk]

Leave it open, the hardest task is to make an svg that has a nested clipPath and that has visual differences with one that doesn't have a nested clipPath. Once we have a reproducible use case of an svg that can render something that works we can add it here and see what happens

I have reopened it now
will see if I can find some svgs that do something useful for these scenarios
or if someone else has them can plop them in here

[stale]

This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs 😔. Thank you for your contributions.

[Smrtnyk]

closing in favor of #10774