give getSrc a better role in this class (tests fix)

[kherock]

Working off of #3173.
I'm seeing if I can get this to pass those couple tests. Also, the conditional for checking if the element existed was preventing using this.src as a fallback.

[kherock]

Ok. I have a pretty good idea of what needs to be done to have this passing, but I have an important question: What is the expected behavior of getSrc in a Node.js environment? The Image element created by node-canvas uses the src property as a setter taking a Buffer or base64 encoded string, and it gives back a Buffer.

For general use and for testing, wouldn't it make more sense if the tests worked with a base64 data URL? The test that the image source is undefined in a Node environment isn't really proving anything.

[asturur]

i did not looked in tests yet.
the point of getSrc is that i noticed that when subclassing you may need to have your own src property. you may have a custom class that work on cropped images based on canvas elements that lack .src support and you do not want to use dataUrl as a source.

[kherock]

Ok, it turns out those tests were really hacked together to get Node.js stuff to pass. It seems like the author of those wasn't aware of how node-canvas's src property worked.

In the Node.js fabric.util.loadImage override, you can see where the _src property is set to preserve the original url passed in. That property should always be the one returned by getSrc (since it's how toObject serializes the image) since it's what was used to create the image!

The tests aren't leveraging the fabric.util.loadImage (should they? it can handle fs paths), and it looks like the author forgot to also set img._src in the test boilerplate. Setting that and adjusting getSrc to check fabric.isLikelyNode seems to make everything behave as intended.

[asturur]

Thanks for looking at tests.
I have still to add some tests to see that getSrc returns what expected.
I will merge this for now.