Fix(SVGParser) ignore missing xlink target issue on svg parsing

[gloriousjob]

Motivation

@closes #9109

Description

Sometimes an svg has use tags with an xlink with no target, which most parsers ignore. We should ignore these use tags as well.

Changes

Adds a null check and returns early, preventing an NPE.
Also removed elementById since it was needed for IE8 support, which we no longer support (see support for everything else here: https://developer.mozilla.org/en-US/docs/Web/API/Document/getElementById).

In Action

[ShaMan123]

This looks good
Thanks!
Can this happen with other directives like clip paths? Should we igonre them as well?

[gloriousjob]

I didn't realize that was being used by the use tag (elements_parser apparently handles this). But I found an example and tested it. It worked normally and when I changed the id to a bad one, it ignored the clipping so looks like the code is already healthy for that.
Only question I have now is how to get the 2 check failures to pass. I'm not sure if this is because I'm using my own repo since I don't have write access on the official repo.

[ShaMan123]

I didn't realize that was being used by the use tag (elements_parser apparently handles this). But I found an example and tested it. It worked normally and when I changed the id to a bad one, it ignored the clipping so looks like the code is already healthy for that.

Could you add a test?

Only question I have now is how to get the 2 check failures to pass. I'm not sure if this is because I'm using my own repo since I don't have write access on the official repo.

You are right, it is because of write access.

[gloriousjob]

Could you add a test?

Tacked on additional test.

[asturur]

Do we have an svg with missing xlink and can we see how the browser renders it?

[gloriousjob]

Do we have an svg with missing xlink and can we see how the browser renders it?

The svg from the unit test looks like (
Pretty simple but without the fix, nothing renders in fabric whereas it renders like this with the fix):

[asturur]

i moved the read shape into the defs element so we can test that the use element with the wrong clip-path still work without the clippath.
If the path is outside defs, you never know if the parser ignored the wrong clip-path attribute or ignored the whole <use> node

[asturur]

@ShaMan123 i m merging this since is a good fix and we can push it up with the beta right away, ok?

[ShaMan123]

sure