Fix path initialize

[sapics]

At #2095, there are two problems inside.
First problem comes from group.toObject would be fixed by #2101.
Second problem comes from path.initialize would be fixed by this PR.

At initialization, option.top = 0, then, path.top is updated by _setPositionDimensions, which makes path.top to nonzero, sometime.

To avoid updating by _setPositionDimensions, this PR set path.top=null if option.top is undefined.
In _setPositionDimensions, judge updating path.top by path.top is null or not. set path.top by option.top is undefined or not.

[asturur]

Ok the point is clear.
we take calcDim left and top just if we did not specified any top left in options.

I would it differently anyway,
i would not check options.left and options.top in the initialize just to give this.left and this.top null.
At that point, after setOptions, those are undefined.
I would pass options to calcPositionDimensions(options)
and check options.left for undefined there instead of this.left and this.top = null.

And this was my first thinking.

Now while i write, i'm asking myself... this is gonna break SVGS.
Did you test SVGS import with this modifications? no nothing.

Do you have a link to the situation that is creating the bug?

[sapics]

@asturur Thank you for your reply!
Sorry, I am not good at English, I wish to confirm your opinion first.

You suggest that
this._setPositionDimensions(options);
and
this.left should be set by options.left is undefined or not.
Is it right?

[asturur]

So we do not add anything in initialize. is useless if you think of it, just send the options in _setPositionDimensions, is less code to write, maybe more readable.
Setting null to top and left, is gonna break everything. I do not feel safe even if i know that 20 rows later this is gonna be fixed by another condition.
Let's just avoid it.

[sapics]

@asturur Absolutely! Thank you for your suggestion!
I fix it.

I write a unit test.

var path = new fabric.Path('M 100 100 L 200 100 L 170 200 z', { top: 0 });
equal(path.left, 100);
equal(path.top, 0);

This was failed at master build, it would not fail after this PR.

Is it better to create jsfiddle?

[sapics]

It would be the situation for confirming the bug.
https://jsfiddle.net/sapics/mq00xq3y/101/

[asturur]

https://jsfiddle.net/mq00xq3y/102/

Ok here is more clear, same Path, same top, just left 0 and left 10 and the path are misplaced.
In your fiddle the path are too much different to give evidence.

Thank you anyway, nasty hidden bug ( created by me for sure )

[sapics]

Thank you for jsfiddle. It is more clear, absolutely!

[asturur]

@kangax this PR is safe.

Long of time ago ( jun 2014 ) i changed path initialize to solve various glitch of svg import and drawing in general. The logic was 'isLeftSet' and 'isTopSet', i changed them with this.top = this.top || calcDim.top ( same logic for left ).
This was obviously wrong because it was discarding user options with top or left set to zero.
Also i was traslating calcDim.left and top to respect originX and originY even in situations where was unecessary because this.top and this.left are already set.

[kangax]

Added to https://github.com/kangax/fabric.js/wiki/Changelog-(draft)