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

SVG group loading failing #5412

Closed
onassar opened this issue Nov 28, 2018 · 16 comments · Fixed by #5424
Closed

SVG group loading failing #5412

onassar opened this issue Nov 28, 2018 · 16 comments · Fixed by #5424

Comments

@onassar
Copy link

onassar commented Nov 28, 2018

Version

Kitchen Sink

Test Case

On Kitchen Sink demo, use the following vector code:

<?xml version="1.0" encoding="iso-8859-1"?>
<!-- Generator: Adobe Illustrator 19.1.1, SVG Export Plug-In . SVG Version: 6.00 Build 0)  -->
<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" version="1.1" id="Layer_1" x="0px" y="0px" viewBox="0 0 48 48" style="enable-background:new 0 0 48 48;" xml:space="preserve">
<circle style="fill:#7986CB;" cx="24" cy="24" r="20"/>
<image style="display:none;overflow:visible;opacity:0.34;" width="87" height="79" xlink:href="8EB19EB6.jpg" transform="matrix(0.6329 0 0 0.6329 -4.0633 -0.5)">
</image>
<polygon style="fill:#E8EAF6;" points="31,17 24,14 17,17 14,24 17,31 24,34 31,31 34,24 "/>
<polygon style="fill:#F3F5FF;" points="31,17 24,14 17,17 14,24 17,31 "/>
<path style="fill:#C5CAE9;" d="M32.515,5.923C29.929,4.703,27.05,4,24,4v5L32.515,5.923z"/>
<path style="fill:#F3F5FF;" d="M15.485,5.923C18.071,4.703,20.95,4,24,4v5L15.485,5.923z"/>
<polygon style="fill:#C5CAE9;" points="14,14 24,14 17,17 "/>
<polygon style="fill:#9FA8DA;" points="14,24 14,14 17,17 "/>
<polygon style="fill:#C5CAE9;" points="15.485,5.923 14,14 24,9 "/>
<polygon style="fill:#F3F5FF;" points="14,14 24,9 24,14 "/>
<polygon style="fill:#F3F5FF;" points="14,24 9,24 14,14 "/>
<polygon style="fill:#C5CAE9;" points="9,24 14,14 5.595,16.167 "/>
<path style="fill:#F3F5FF;" d="M14,14L9.858,9.858c-1.797,1.797-3.252,3.936-4.263,6.309L14,14z"/>
<path style="fill:#9FA8DA;" d="M9.858,9.858L14,14l1.485-8.076C13.387,6.913,11.478,8.237,9.858,9.858z"/>
<path style="fill:#9FA8DA;" d="M5.595,16.167C4.57,18.573,4,21.219,4,24h5L5.595,16.167z"/>
<path style="fill:#F3F5FF;" d="M5.923,32.515C4.703,29.929,4,27.05,4,24h5L5.923,32.515z"/>
<polygon style="fill:#9FA8DA;" points="14,34 14,24 17,31 "/>
<polygon style="fill:#C5CAE9;" points="24,34 14,34 17,31 "/>
<polygon style="fill:#9FA8DA;" points="5.923,32.515 14,34 9,24 "/>
<polygon style="fill:#C5CAE9;" points="14,34 9,24 14,24 "/>
<polygon style="fill:#C5CAE9;" points="24,39 14,34 16.167,42.405 "/>
<path style="fill:#9FA8DA;" d="M14,34l-4.142,4.142c1.797,1.797,3.936,3.252,6.309,4.263L14,34z"/>
<path style="fill:#C5CAE9;" d="M9.858,38.142L14,34l-8.076-1.485C6.913,34.613,8.237,36.522,9.858,38.142z"/>
<polygon style="fill:#9FA8DA;" points="34,14 24,14 31,17 "/>
<polygon style="fill:#C5CAE9;" points="34,24 34,14 31,17 "/>
<polygon style="fill:#E8EAF6;" points="32.515,5.923 34,14 24,9 "/>
<polygon style="fill:#C5CAE9;" points="34,14 24,9 24,14 "/>
<polygon style="fill:#9FA8DA;" points="34,24 39,24 34,14 "/>
<path style="fill:#C5CAE9;" d="M34,14l4.142-4.142c1.797,1.797,3.252,3.936,4.263,6.309L34,14z"/>
<path style="fill:#9FA8DA;" d="M38.142,9.858L34,14l-1.485-8.076C34.613,6.913,36.522,8.237,38.142,9.858z"/>
<path style="fill:#9FA8DA;" d="M42.405,16.167C43.43,18.573,44,21.219,44,24h-5L42.405,16.167z"/>
<polygon style="fill:#C5CAE9;" points="34,34 34,24 31,31 "/>
<polygon style="fill:#9FA8DA;" points="42.077,32.515 34,34 39,24 "/>
<polygon style="fill:#9FA8DA;" points="24,34 24,39 34,34 "/>
<path style="fill:#9FA8DA;" d="M34,34l4.142,4.142c-1.797,1.797-3.936,3.252-6.309,4.263L34,34z"/>
<path style="fill:#9FA8DA;" d="M31.833,42.405C29.427,43.43,26.781,44,24,44v-5L31.833,42.405z"/>
</svg>

Information about environment

Chrome Browser

Steps to reproduce

Copy and paste the above vector code into the Kitchen Sink demo (loading a vector).

Expected Behavior

It works.

Actual Behavior

It doesn't, and produces this error in the console:

@rikkarv
Copy link

rikkarv commented Dec 5, 2018

What it is the purpose of that image tag?
Anyway, i think the problem is that fabric does not know where is the image. I believe that xlink:href='8E819EB6.jpg' does not work. Try to replace the source of image with a dataurl

@onassar
Copy link
Author

onassar commented Dec 5, 2018

I'm not sure what the purpose is, but should Fabric be able to parse this correctly, since Chrome does? Perhaps it should strip out <image> tags that have references to files?

@rikkarv
Copy link

rikkarv commented Dec 5, 2018

Maybe you should have something like xlink:href='data:image/JPEG;base64,...'

@rikkarv
Copy link

rikkarv commented Dec 5, 2018

Your Chrome does parse because he can reach the image file that it is outside the svg. However, anyone who tries to see that svg in chrome cannot see that image

@rikkarv
Copy link

rikkarv commented Dec 5, 2018

Please try http://freeonlinetools24.com/base64-image
Drop your image, click encode and copy that string to replace your href.
Finally try to parse the svg in fabric

@onassar
Copy link
Author

onassar commented Dec 5, 2018

The file doesn't actually exist; it's a bug inside the SVG from a user-submitted upload. I think fabric should filter out image tags on image values that it can't reach?

@rikkarv
Copy link

rikkarv commented Dec 5, 2018

I don't kown, sorry

@asturur
Copy link
Member

asturur commented Dec 8, 2018

yes fabric should not die and give you a blank image object.
Not filter it out. But if you know that this may happen you can take care of it in your load callback

@asturur asturur added the bug label Dec 8, 2018
@onassar
Copy link
Author

onassar commented Dec 8, 2018 via email

@asturur
Copy link
Member

asturur commented Dec 8, 2018

yes error should not happen.

@onassar
Copy link
Author

onassar commented Dec 8, 2018 via email

@asturur
Copy link
Member

asturur commented Dec 9, 2018

the main problem is that here we are parsing that attribute as a number, 8.
I m trying to figure out why and how.

@asturur
Copy link
Member

asturur commented Dec 9, 2018

i think is safe to assume that handling xlink:href and href as a special case that does not need to pass in parseUnit is safe enough. Could look patchy, but at least is explicit and coherent with what we do for other non number properties.

@onassar
Copy link
Author

onassar commented Dec 9, 2018 via email

@asturur
Copy link
Member

asturur commented Dec 9, 2018

well that was handled and failed later. it was failing before because we were using parseFloat on that url that was returning '8' as a number and throwing on .indexOf

@onassar
Copy link
Author

onassar commented Dec 9, 2018 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants