Skip to content

Conversation

@Smrtnyk
Copy link
Contributor

@Smrtnyk Smrtnyk commented Oct 8, 2025

another attempt to solve #10667 but now with visual tests as well

fixes #10659

@codesandbox
Copy link

codesandbox bot commented Oct 8, 2025

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@github-actions
Copy link
Contributor

github-actions bot commented Oct 8, 2025

Build Stats

file / KB (diff) bundled minified
fabric 917.639 (+0.858) 301.990 (+0.279)

@asturur
Copy link
Member

asturur commented Oct 22, 2025

Looking at this.

@Smrtnyk
Copy link
Contributor Author

Smrtnyk commented Oct 22, 2025

is firefox flaky again?

@asturur
Copy link
Member

asturur commented Oct 31, 2025

I spent some hour on this PR, i modified one example two highlight 2 issues:

  1. all the svg import test after migration do not behave correctly.
  2. This works but it doesn't cover transformation.

For 1) i will open an issue and the problem is that the test now compare the rendering on browser and node of an imported svg, but it does not compare it to the actual svg. So all our svg import tests now are kind of useless.

for 2) i have some test to do in mind, hopefully i can get back to it tonight. I may have accidentally committed some half backed idea in the code now.

@asturur
Copy link
Member

asturur commented Oct 31, 2025

This is the svg
image

this is the snapshot and it passes

image

@Smrtnyk
Copy link
Contributor Author

Smrtnyk commented Oct 31, 2025

For 1) i will open an issue and the problem is that the test now compare the rendering on browser and node of an imported svg, but it does not compare it to the actual svg. So all our svg import tests now are kind of useless.

oh crap, that doesn't sound good
but that issue was there even before migration, right? because there were alraedy some tests written in playwright before we commited to this full migration

@asturur
Copy link
Member

asturur commented Oct 31, 2025

For 1) i will open an issue and the problem is that the test now compare the rendering on browser and node of an imported svg, but it does not compare it to the actual svg. So all our svg import tests now are kind of useless.

oh crap, that doesn't sound good but that issue was there even before migration, right? because there were alraedy some tests written in playwright before we commited to this full migration

I do not know when it broke. I think before the svg test had some code that was rendering it as an img and on the canvas and using that as reference. But it doesn't matter is going to be fixed.

const clipPathTag = clipPathElements[0].parentElement!;
const clipPathTag = clipPathElements[0].parentElement!.cloneNode(
true,
) as HTMLElement;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is new from me, i wanted to create a copy of the clipPath since we are adding to it transforms attributes depending on the element using it

`matrix(${finalTransform.join(',')})`,
);

const updatedProcessedClipPaths = new Set(processedClipPaths);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you remember why the new set each time?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hm, don't really remember now
might be a brain fart that I had to do it

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.

[Bug]: nested duplicated clipPath causes infinite recursion

3 participants