matiasgali / guillotine

jQuery plugin to crop images within an area (fully responsive), allowing to drag (touch support), zoom and rotate.
http://guillotine.js.org
326 stars 100 forks source link

Doesn't seem to work on iPhone in either Safari nor Chrome #15

Closed loevgaard closed 9 years ago

loevgaard commented 9 years ago

I have tried your demo on iPhone in both Safari and Chrome. In Chrome the picture doesn't display and in Safari I can't zoom, rotate or anything.

matiasgali commented 9 years ago

Thanks, I really appreciate the feedback.

Some time ago a friend pointed out the same problem with Chrome on his iPad, unfortunately it was impossible to debug it inside the iPad, I believe developer tools or anything similar weren't available.

For the moment I don't have an iPad or iPhone at hand but I'll see if I can debug it using some service like Sauce Labs, but still without a console it won't be easy so any further information that you can provide will be much appreciated.

veewee commented 9 years ago

We are facing the same issue.

czachor commented 9 years ago

Same problem here - I'm uploading an image, trying to init Guillotine, but nothing happens in Safari (works in FF, Chrome, not tested in IE).

The problem lies in Guillotine.prototype._wrap, line:

_ref = [img.width, img.height], width = _ref[0], height = _ref[1];

In Safari, both img.width and img.height are empty (0). But if you put this line into img.load(), they will have correct values.

My dirty solution:

    if (img.width > 0) {
        _ref = [img.width, img.height], width = _ref[0], height = _ref[1];
    } else {
        _ref = [this.op.imgWidth, this.op.imgHeight], width = _ref[0], height = _ref[1];
    }
    imgWidth: 0,
    imgHeight: 0

and use them instantiating the plugin. Finally, my Guillotine init looks like:

$(image).on('load', function(){
    var t = $("#the-image");

    t.guillotine({
        eventOnChange: "guillotinechange",
        width: 700,
        height: 700,
        imgWidth: t.get(0).naturalWidth,
        imgHeight: t.get(0).naturalHeight
    });
});
xims commented 9 years ago

@czachor - your solution is working great, thanks for sharing and explaining it well. This library now works on desktop Safari and on iPad! @matiasgagliano - would be nice to see it merged into library code. Thanks for a great library!

matiasgali commented 9 years ago

It's coming, I've been very short on time lately but I been refactoring the plugin and working on some improvements whenever possible.

I think it can be solved by using img.naturalWidth and naturalHeigth, and falling back to img.width and height when not supported. I still have a lot of testing ahead to determine if that leaves any gaps (browsers not supporting naturalWidth nor width). Any contribution about testing this will be much appreciated.

Please be patient for a while.

xims commented 9 years ago

I'd be happy to help with testing, please let me know how can I help.

Sedins commented 9 years ago

As a senior developer who really needs this to work I'm also interested in helping out with testing the issue.

I got access to iPhone 4, 5S and 6+ and being able to test it on a variety of IE, Chrome, Firefox and on Android KitKat and Lollipop.

Please contact me asap.

matiasgali commented 9 years ago

Hi all, I'm deeply sorry that this has taken so long, unfortunately I haven't been able to access all the devices I would have wanted.

Anyway, to avoid further delays I've pushed what in theory should be a definitive fix, to a branch called "issue-15".

@Sedins, @xims and anyone else willing to give a hand with testing, please try the demo at issue-15 branch.

After testing, it'd be much appreciated if you could update the support page of the wiki with your results in case they are positive and report the problems/issues here so we can address them.

Thank you very much for your support!

Sedins commented 9 years ago

Ran some initial tests with the issue-15 branch.

Chrome v39 on Windows 7 and 8.1 works as expected, but dragging the image seems broken in IE11 (it doesn't move). All remaining functionally seem to work though (zoom, rotate, fit). No errors in console neither. Running site in IIS on a Win 7 developer VM. Do anyone else get the same result?

Edit: Also ran the demo with IIS 7.5 on Win 2008 R2 Server, but unable to get the image dragging to work. Tried both the master and issue-15 braches without success. Maybe this problem is unrelated to the Iphone one in this thread after all.

Wanted to make sure this works as previous versions before continuing testing on mobiles.

Minor problem: Sometimes (rarely) the plugin does not seem to initialize when starting or refreshing the site in IE. All values below the image, including x and y, are empty. This happen for me with the official demo as well. Probably just a timing problem on init.

Sedins commented 9 years ago

Issue-15 branch tests:

iPhone 5 running iOS 7 using Safari works. iPhone 5 running iOS 7 using Chrome does not display the image. Nexus 5 running Android 5.0 using Chrome works. PC running Windows 7 and 8.1 using Chrome 39 works. PC running Windows 7 and 8.1 using IE 11 works except for dragging image which is broken. PC running Windows 2008 R2 using IE 10 works.

All tests done using xims test site at http://www.dev.kislanet.com/guillotine-15/demo/

xims commented 9 years ago

For the convenience of other testers I've put branch-15 online. You can see it at http://www.dev.kislanet.com/guillotine-15/demo/

It works in Chrome and Safari on Windows. It works on Chrome on Android. It does works in Safari on iPad But it doesn't works in Chrome on iPad - it displaying the image while it's loading but when image is loaded it's dissapears.

matiasgali commented 9 years ago

I've pushed a more robust approach to the issue-15 branch, I hope it fixes the issue with Chrome.

I guess that Chrome for iPhone and iPad supports neither img.naturalWidth and img.naturalHeight nor img.width and img.height.

According to this desktop support for natural properties is quite good, but probably mobile not so much. It seems unthinkable that the same browser wouldn't support the same properties across devises, there's something weird cooking here.

@Sedins, I haven't been able to reproduce the issue about the plugin not initializing, could you elaborate more about it? Does the image load?

The dragging issue on IE11 most likely relates to pointer events, I'll see into it after working this out.

Thanks for contributing!

Sedins commented 9 years ago

No problem. Here goes about uninitialized cropper:

The image does load but is displayed as a normal image. It's not possible to drag it just as if the cropper hasn't been initialized. All values (x/y/width/height/scale/angle) are empty as well. Refreshing the page one or two times use to resolve the problem, but that is not pretty (most users may not even do it)...

It reproduces every 2-3 refresh in IE 11 with the same result on both the official and another IIS running the demo site. Clients used were a Windows 7 in Hyper-VM and a stand-alone Windows 8.1 desktop machine. I very rarely have seen the same problem in Chrome.

Console is free from errors, but displays this warning in Chrome (in IE it's empty): The key "target-densitydpi" is not supported. Pretty sure that it's not related, but mentioning it anyway.

Edit: Could be related to pointer events as you said, but it's working most of the times which makes it weird. The x/y-values are empty as well. I haven't checked how the plugin checks for different browsers, but IE 11 is responding with "Mozilla" as user-agent in the headers nowadays.

Chrome on Windows 7: Mozilla/5.0 (Windows NT 6.1; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/38.0.2125.111 Safari/537.36

IE 11 on Windows 7: Mozilla/5.0 (Windows NT 6.1; WOW64; Trident/7.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; .NET4.0C; .NET4.0E; rv:11.0) like Gecko

Have not dug into the code and stuff, but my feeling it that it's a timing problem that makes the cropper initialize itself before the DOM is ready or something similar.

Related note: Have seen the same behavior in another cropbox as well. The problem was that the cropbox initialized before the image was loaded. They solved it by creating an onload event on the plugin where it was possible to trigger to initialization routine from.

Attached is an image showing me dragging the image (which only moves the whole image, not dragging in within the crop container) and empty x/y-values.

cropper_empty_values

Sedins commented 9 years ago

Current issue-15 branch tests:

iPhone 6+ running iOS 8 using Safari works! iPhone 6+ running iOS 8 using Chrome works! Nexus 5 running Android 5.0 using Chrome works. PC running Windows 7 and 8.1 using Chrome 39 works. PC running Windows 2008 R2 using IE 10 works.

Sedins commented 9 years ago

Current issue-15 branch tests:

iPhone 5 running iOS 7 using Safari works! iPhone 5 running iOS 7 using Chrome works!

matiasgali commented 9 years ago

Thank you very much @Sedins.

I'll push this browser compatibility fixes to master with a few minor improvements and wrap the 1.3 version to avoid delaying it further.

After that it'd be nice if you and/or @xims could update the wiki's support page so you're properly credited for your contributions.

I've confirmed that the dragging issue on IE11 is due to pointer events. They trigger mouse events anyway so I'll drop support for pointer events for now.

Given that the initialization issue happens mostly on IE and that it isn't critical I'll leave it for the next mayor release. Many reported troubles initializing the plugin because they didn't take into account that the image needs to be completely loaded before running the plugin, so I'm planning to re design some sections to ease this out, possibly this will fix the initialization issue too.

Sedins commented 9 years ago

Sounds great! Also dropping support for pointer events as a workaround for now would be nice to get the plugin working across all browsers.

Thanks a lot for this plugin. Looking forward to the 1.3 release. Really no problem to help out with the testing part a bit as we all benefit from it. Always nice to hear that it's appreciated though. I've updated the support page with new entries which has been tested btw.