search

goat1000 / TagCanvas

HTML5 canvas-based tag cloud
243 stars 81 forks source link

Added image error handling #6

Open ytyukhnin opened 9 years ago

ytyukhnin commented 9 years ago

Hi could you please merge this small change? I think it's useful. Thank you.

If an image cannot be downloaded it throws lots of exceptions during drawing, added onError handler to the images solves this issue.

goat1000 commented 9 years ago

I've just done some testing with this, and it's not very clear to me how this change helps. If you add an error handler to the images to fix the image, then it looks like TagCanvas can used the fixed image without your modification.

If the error handler doesn't update the image, then TagCanvas will still have a broken image and the change won't help.

I can see how adding an error handler to TagCanvas would be useful, but I think it might be better to make TagCanvas deal with the error itself instead of using a handler from the image element.

Do you have any examples of where this change is really useful?

ytyukhnin commented 9 years ago

Hi,

Thank you for your reply. A use case is exactly that you'd like to update your image in the error handler.

Here is it:

I have my images hosted out of my control, so some of them are OK, some are no longer available (404), some are hosted on some crappy servers (500) or sent with an incorrect content type. I'm generating a list of As and IMGs dynamically:

Broken or unavailable images while drawn on the canvas are generating an infinite number of errors, hanging all the UI.

image

With my update it's easy to handle

I'd like to display another image in place of the broken one (while additionally keeping it possible to provide a tooltip and other info which might be useful to display)

The update I'm proposing helps to handle this use case while keeping the TagCloud behaviour as it was if no error handler was provided.

Hope this example is clear enough. Should you have any questions, please don't hesitate.

Thank you.

2014-11-07 11:30 GMT+01:00 goat1000 notifications@github.com:

I've just done some testing with this, and it's not very clear to me how this change helps. If you add an error handler to the images to fix the image, then it looks like TagCanvas can used the fixed image without your modification.

If the error handler doesn't update the image, then TagCanvas will still have a broken image and the change won't help.

I can see how adding an error handler to TagCanvas would be useful, but I think it might be better to make TagCanvas deal with the error itself instead of using a handler from the image element.

Do you have any examples of where this change is really useful?

— Reply to this email directly or view it on GitHub https://github.com/goat1000/TagCanvas/pull/6#issuecomment-62124652.

goat1000 commented 9 years ago

I've tried out the onerror handler, and it replaces the broken image with a new one as expected.

But the extra line in TagCanvas doesn't appear to make any difference. Without the "onerror=" in the img element, TagCanvas doesn't work with or without the extra line. With the "onerror=" in the img element it does work with or without the extra line.

I'm just trying to understand under what conditions it makes a difference to have the new line of code. Is it only required for specific browsers, or when it takes a long time for the image to fail?

ytyukhnin commented 9 years ago

Hi,

Sorry for this delay. Actually I was trying to reproduce it using jsfiddle but I cannot, seems that you are right about that line has no effect. I'll check this change one again where it was used get back to you.

Thank you.

2014-11-11 16:38 GMT+01:00 goat1000 notifications@github.com:

I've tried out the onerror handler, and it replaces the broken image with a new one as expected.

But the extra line in TagCanvas doesn't appear to make any difference. Without the "onerror=" in the img element, TagCanvas doesn't work with or without the extra line. With the "onerror=" in the img element it does work with or without the extra line.

I'm just trying to understand under what conditions it makes a difference to have the new line of code. Is it only required for specific browsers, or when it takes a long time for the image to fail?

— Reply to this email directly or view it on GitHub https://github.com/goat1000/TagCanvas/pull/6#issuecomment-62564496.