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

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

Closed
loevgaard opened this issue Nov 13, 2014 · 17 comments
Closed

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

loevgaard opened this issue Nov 13, 2014 · 17 comments
Labels

Comments

@loevgaard
Copy link

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
Copy link
Owner

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
Copy link

veewee commented Nov 27, 2014

We are facing the same issue.

@czachor
Copy link

czachor commented Dec 2, 2014

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:

  • replace line with:
    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
Copy link

xims commented Dec 12, 2014

@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
Copy link
Owner

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
Copy link

xims commented Dec 13, 2014

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

@Sedins
Copy link

Sedins commented Dec 28, 2014

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
Copy link
Owner

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
Copy link

Sedins commented Dec 29, 2014

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
Copy link

Sedins commented Dec 29, 2014

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
Copy link

xims commented Dec 29, 2014

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
Copy link
Owner

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
Copy link

Sedins commented Jan 2, 2015

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
Copy link

Sedins commented Jan 2, 2015

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
Copy link

Sedins commented Jan 3, 2015

Current issue-15 branch tests:

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

@matiasgali
Copy link
Owner

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
Copy link

Sedins commented Jan 4, 2015

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants