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

FabricJS loadFromJSON() - is there any way to ignore images that their sources can not be found? #3578

Closed
artboard-studio opened this issue Dec 23, 2016 · 16 comments · Fixed by #3586

Comments

@artboard-studio
Copy link

artboard-studio commented Dec 23, 2016

Version

1.6.7 and 1.7.2

Test Case

https://jsfiddle.net/human_a/rs8cbxv8/

Steps to reproduce

My canvas project is saved in the database as a JSON object. Some of these projects have images being added to them from a user uploaded library of images. The problem is that if one of the images in the library is deleted, the entire project would not be loaded, so I am looking for a way to load the rest of the objects into the canvas and ignore the ones that can not be loaded.

After trying to add this issue to a jsfiddle I realized that the loading of the canvas fails only and only if a filter is set on the image.

I make my images ready to have filters applied to them while the images are being added to the canvas using the following method:

var filter = new fabric.Image.filters.Brightness({
         brightness: null
})
newImage.filters.push(filter);
newImage.applyFilters(canvas.renderAll.bind(canvas))

In the jsfiddle inside the JSON object try changing "src":"https://fake.fakedoman.fake/fake.jpg" to a valid image URL, click on load button and it will start working correctly. Also if you remove "filters":[{"brightness":0,"type":"Brightness"}] array from the JSON object the loading will work again and just ignores the image with a fake source.

@suncha-ng
Copy link

suncha-ng commented Dec 23, 2016

I` perform this server-side action in PHP before injecting the JSON stream into the canvas.

public static function sanitize($json){
		
		$i = 0;
		$j = 0;
		
		foreach($json->pages as $pages){
			foreach($pages->value->objects as $object){
				if($object->type === 'image'){
					$file_headers = @get_headers($object->src);
					if($file_headers[0] === 'HTTP/1.0 404 Not Found'){
					   unset($json->pages[$i]->value->objects[$j]);
					}
				}
				$j++;
			}
			$i++;
		}
		
		return $json;
		
	}

@asturur
Copy link
Member

asturur commented Dec 23, 2016

i think this error should be handled at least in the reviver function passing the error so that you can decide what to do.
We are managing the error in fabric that is custom logged, so we should be able to fix this.

@asturur
Copy link
Member

asturur commented Dec 26, 2016

Can you recreate the fiddle? there is the fiddle from another issue but not the original one.

@asturur
Copy link
Member

asturur commented Dec 26, 2016

I made a fiddle by myself and the canvas loads without problem, just the wrong object is missing. Are you sure there is not some other problem? I agree that the reviver is not called in case of error so you cannot remove the empty object from canvas.

@artboard-studio
Copy link
Author

I edited the question with the correct link

I tested it with different images, in different situations, if you apply filters (like brightness e.g.) it would not load any other objects into the canvas at all.

"In the jsfiddle inside the JSON object try changing "src":"https://fake.fakedoman.fake/fake.jpg" to a valid image URL, click on load button and it will start working correctly. Also if you remove "filters":[{"brightness":0,"type":"Brightness"}] array from the JSON object the loading will work again and just ignores the image with a fake source."

@asturur
Copy link
Member

asturur commented Dec 26, 2016

ok so there is an unmanaged error inside the loadFromObject in case of filters

@asturur
Copy link
Member

asturur commented Dec 26, 2016

https://github.com/kangax/fabric.js/pull/3586/files those changes should solve this defect

@artboard-studio
Copy link
Author

That is great. Thanks!

@artboard-studio
Copy link
Author

I just gave it a try, If I do the changes in the first file on the link you sent me Everything works as expected, however by doing the second change:

enlivenedObjects[index] = obj;
reviver && reviver(o, enlivenedObjects[index], error);

It stops loading all the other objects and it would not set the background color, size etc. on the canvas giving the error of "can not run set of null"

Just wanted to inform you before you publish the changes in the new version.

@asturur
Copy link
Member

asturur commented Dec 27, 2016

i should not trust unit tests alone. all give a better look.

@asturur
Copy link
Member

asturur commented Dec 27, 2016

ok i avoid to set the null object in the returned array, as it was before. Still calling the reviver with error so that a dev can figure out wich object errored.

@artboard-studio
Copy link
Author

@asturur I am experiencing the same issue with groups. Any idea how to resolve that one too?

@asturur
Copy link
Member

asturur commented Jan 7, 2017

last time you gave a fiddle and you got help and solution. it worked.
why not coming back with a fiddle directly?

@artboard-studio
Copy link
Author

@asturur : You are right. However, for some reason, I can not replicate this issue in a fiddle, so this must be caused by something else, although the error that I get in the console is exactly the same as before. I will keep you posted here as soon as I have a presentable fiddle.

@artboard-studio
Copy link
Author

Here is a working fiddle: https://jsfiddle.net/human_a/63wukwhu/

In addition to the error you can see in the console, I also get the following error on my local machine:

fabric.js:17967 Uncaught TypeError: Cannot set property 'group' of undefined
    at klass.initialize (fabric.js:17967)
    at klass.eval [as initialize] (fabric.js:1745)
    at new klass (fabric.js:1793)
    at eval (fabric.js:18450)
    at onLoaded (fabric.js:674)
    at eval (fabric.js:700)
    at eval (fabric.js:19098)
    at HTMLImageElement.img.onerror (fabric.js:644)

@artboard-studio
Copy link
Author

@asturur This issue is still not fixed for groups. I have provided a working fiddle for it too. Please read the reply above this one.

Any ideas?

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.

2 participants