Image filters in Safari cause image to not render with certain crop values

[melchiar]

Version

3.6.3

Test Case

https://jsfiddle.net/melchiar/gdo2szay/

Information about environment

Safari - Tested on iPad

Steps to reproduce

Add an image. Set a cropX value greater than (img._originalElement.width - img.width). Apply a filter. The same applies with cropY, and this happens with both filters and resizeFilters.

Expected Behavior

The image should render.

Actual Behavior

When using Safari, applying a filter makes the image disappear. It works as expected in Chrome and Firefox.

[asturur]

ok i think we can see if it is easily fixable. But since you set cropX and cropY with the code, and not with the user mouse events, what stops you from setting correct values?

[melchiar]

Nothing at all. I happened upon the bug due to a rounding error in my application that caused some images to have a crop value 1px too large. Now that I know it's there I can easily correct for it, but the bug actually went unnoticed in my application for a long time since I don't typically develop on a Mac. Ideally the behavior should be consistent across all browsers to avoid this type of thing.

[asturur]

ie11 is another browser that will freak out on drawImage with wrong values.

[melchiar]

Would it be better to just cap cropY and cropY to appropriate max values in a future release? This might be classed as a breaking change though since I imagine some applications might intentionally allow larger crop values in order to achieve an image container effect

[asturur]

those values have been always meant to be regulated, indeed image render method has already some fix of this kind. In my first comment the first sentence was indeed toward fixing.

[asturur]

is very hard to understand why the bug happens with filtering only.

this is a possible fix:

the rendering kind of make sense since the dimension of the image are off, we do not expect a correct rendering.

[asturur]

kind of forget about this issue
I have an open PR if tests pass i ll merge it.

[asturur]

fixed with #6326