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

fixed bug when loadFromDatalessJSON does not load text objects #144

Merged
merged 1 commit into from Mar 28, 2012

Conversation

sunrei
Copy link
Contributor

@sunrei sunrei commented Mar 23, 2012

this is a fix for issue #143

@kangax
Copy link
Member

kangax commented Mar 26, 2012

This is pretty strange. I see that the code is not supposed to work (as you explained in the commit), but it actually does work in my project. We're also using loadFromDatalessJSON and text objects always load just fine. I wonder what's going on there...

@kangax
Copy link
Member

kangax commented Mar 26, 2012

Ah... the difference in your JSON and mine is that I have "path" property set to a string (url of a font file), whereas yours is null. That's why my loadFromDatalessJSON goes into a different branch, in which the callback is invoked.

@kangax
Copy link
Member

kangax commented Mar 26, 2012

Why we have "path" property on text objects in our app is not clear at all :)

@sunrei
Copy link
Contributor Author

sunrei commented Mar 26, 2012

If you are talking about printio app then I cant help you there. I see only minified version of sources and its very hard to debug it. I just see that your serilized text objects do have 'path', but I'm not sure that path property is being added automatically by fabric.js. At least in version 0.6.9 it's not so.
You are using different wrappers and I'm almost sure that creating(inserting) text objects into canvas is handled by one that adds path property.
Anyway my fix will not touch the loading of text objects that have 'path' property set, but it will definitely help in other cases.

@kangax
Copy link
Member

kangax commented Mar 26, 2012

Yeah, definitely. Thanks for finding and patching it! I'll merge it.

Just wanted to make sure unit tests are all good after this change. Also need to add another unit test for this particular case. Will do as soon as I get a chance.

kangax added a commit that referenced this pull request Mar 28, 2012
Fixed bug with `loadFromDatalessJSON` not loading text objects when those objects have no "path" property.
@kangax kangax merged commit c551e1b into fabricjs:master Mar 28, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants