Bug 130127 – Nested use of same SVG resource fails

• Status: RESOLVED
• Resolution: FIXED
• Component: SVG

[Dirk Schulze]

Created attachment 226500 [details]
Example - passes if you see a circle

An element is filled with a pattern #p1. The element gets masked as well. If this mask makes use of #p1 as well, then it fails.

This is probably a problem with the cycle detection code for SVG resources. For some reason the code detects a cycle even though this is absolutely fine. Creating a second pattern with a different id makes the example work.

The example passes if you see a circle.

[Ahmad Saleem]

It is still happening with Safari 16.2 and Safari Technology Preview 160, it was fixed in Blink with following commit:

Commit - https://src.chromium.org/viewvc/blink?view=revision&revision=175129 & https://src.chromium.org/viewvc/blink?revision=174862&view=revision

WebKit Source (Example) - https://searchfox.org/wubkat/source/Source/WebCore/rendering/svg/SVGResourcesCycleSolver.cpp#53

Just wanted to share this. Might be bit out of my area of expertise to try to merge this and don't want to cause collision with on-going LBSE work as well (if it reaches in this realm). Just wanted to share. Thanks!

[Tim Nguyen (:ntim)]

> Might be bit out of my area of expertise to try to merge this and don't want to cause collision with on-going LBSE work as well (if it reaches in this realm).

I don't think there is any collision with LBSE in this case. LBSE does not affect resource resolution.

[Ahmad Saleem]

(In reply to Ahmad Saleem from comment #1)
> It is still happening with Safari 16.2 and Safari Technology Preview 160, it
> was fixed in Blink with following commit:
> 
> Commit - https://src.chromium.org/viewvc/blink?view=revision&revision=175129
> & https://src.chromium.org/viewvc/blink?revision=174862&view=revision
> 
> WebKit Source (Example) -
> https://searchfox.org/wubkat/source/Source/WebCore/rendering/svg/
> SVGResourcesCycleSolver.cpp#53
> 
> Just wanted to share this. Might be bit out of my area of expertise to try
> to merge this and don't want to cause collision with on-going LBSE work as
> well (if it reaches in this realm). Just wanted to share. Thanks!

We only need to do: https://src.chromium.org/viewvc/blink?view=revision&revision=175129

^ I tried but I think our code has also evolved, so it is not 1-1, there are still some similarities but we have WeakHashSet intro in this code as well.

@Ryosuke did some WeakHashSet intro in various places of code. Might be something you can have a look?

[Ryosuke Niwa]

Pull request: https://github.com/WebKit/WebKit/pull/14338

[EWS]

Committed 264618@main (99265dce4d2f): <https://commits.webkit.org/264618@main>

Reviewed commits have been landed. Closing PR #14338 and removing active labels.

[Radar WebKit Bug Importer]

<rdar://problem/109917889>