Various issues in IE8

[DanTup]

The readme says IE8 is supported, but we're having issues making this work. Even the demo page is broken in IE8. The textboxes all say NaN, and the dragging has strange behaviour.

[DanTup]

Seems to be this code where things start going wrong:

$clone.one('load', $.proxy(function () {
  var naturalWidth = $clone.prop('naturalWidth') || $clone.width(),
      naturalHeight = $clone.prop('naturalHeight') || $clone.height();

  this.image = {
    naturalWidth: naturalWidth,
    naturalHeight: naturalHeight,
    aspectRatio: naturalWidth / naturalHeight,
    rotate: 0
  };

Both of the variables end up being 0. Thought strangely, while debugging, as soon as I start looking in $clone in the dev tools, it magically gets the correct value for .width()!

[fengyuanchen]

You are right! Thank you! Here is a quick solution:

...
var image = $clone[0],
    naturalWidth = image.naturalWidth || image.width,
    naturalHeight = image.naturalHeight || image.height;
...

[DanTup]

Great; will give it a go. Thanks!

[DanTup]

Thanks, this does solve the number issue. Still seem to be a few other issues in IE8:

1. The bottom-right grip handle is massive
2. Parts of the image outside of the crop do not render at all
3. Example page layout seems broken (everything seems to be full width!)

[DanTup]

Would you accept a PR with the code you pasted here for now? Would like to put this fix in, but don't want to patch our copy if a future update might revert the fix.

[fengyuanchen]

Thank you! But I had fixed it in my local repo and will release it in the next version.

[DanTup]

Great, thanks!

[DanTup]

It's been a while, but I just got around to trying to update this. I took the latest build (from about a week ago?) and it's broken in IE8 still/again :(

I tested the test page here, and it has the same issues.

The homepage suggests you're still supporting IE8 - is this the case?

[fengyuanchen]

You are right. I might miss something. v0.10.1 stiil works on IE8.

[fengyuanchen]

I see. I fix an event issue on IE10 (#394), but cause another issue on IE8...

[DanTup]

Still having issues; just trying to debug. While doing so, I noticed the demo page throws JS errors in IE8 (coming from "carbon.js", possibly ad scripts?)

The other issues might be our code rather than the cropper, but will post back if I think there are still issues here.

[DanTup]

I'm trying to debug our IE8 issues.. The first problem seems to be that a lot of the values (such as the width from getCanvasData) come back as NaN.

I tracked this back to in the start() method, getImageData() is called, and it returns an aspectRatio of NaN.

This seems to be caused by the code shown here - newImage.width and newImage.height are both undefined (line 148 + 149). That means line 155 calculates NaN for aspectRatio.

I'm not sure why I can't cause an equivalent bug in your test page though. It seems like setting .src on newImage doesn't cause it to have a width or height (though it does have originalWidth and originalHeight which appear correct).

[DanTup]

My guess is that width and height aren't available until the image is "loaded" (which is not synchronous even though at it could probably be satisfied directly from the loaded version in the page), but I can't see an easy way to address it without changing this code quite a bit (to hook the onload function; but then this method can't directly return data).

[fengyuanchen]

Yes, should bind onload event handler:

function getNaturalSize(image, callback) {
  var newImage;

  // Modern browsers
  if (image.naturalWidth) {
    return callback(image.naturalWidth, image.naturalHeight);
  }

  // IE8
  newImage = new Image();

  newImage.onload = function () {
    callback(this.width, this.height);
  };

  newImage.src = image.src;
}

[DanTup]

Great; I'll try and give it a test this week and see if it's all working as expected now. Thanks!

[fengyuanchen]

Thank you for reporting these bugs, you really did a great work!

[DanTup]

Np; thanks for fixing up the issues :)

I cloned your repo to try and make a build with this latest change for testing, but when I run npm install to restore dependencies, I get this in the output:

npm ERR! 404 Not Found
npm ERR! 404
npm ERR! 404 'fengyuanchen/tooltip' is not in the npm registry.
npm ERR! 404 You should bug the author to publish it
npm ERR! 404 It was specified as a dependency of 'cropper'
npm ERR! 404
npm ERR! 404 Note that you can also install from a
npm ERR! 404 tarball, folder, or http url, or git url.

npm ERR! System Windows_NT 6.3.9600
npm ERR! command "C:\\Work\\Utilities\\Node\\\\node.exe" "C:\\Work\\Utilities\\Node\\node_modules\\npm\\bin\\npm-cli.js"
 "install"
npm ERR! cwd C:\Work\Source\Cropper
npm ERR! node -v v0.12.3
npm ERR! npm -v 1.4.9
npm ERR! code E404

I don't seem to be able to get gulp to run (says not installed), which I think might be related to npm install failing.

Any chance you could do a build with these latest changes (even if not a normal release, just provide the dist folder I can use)?

[fengyuanchen]

Sorry for this, please try again later. I think the files in the dist directory should not be changed until release a new version.

[DanTup]

Sorry, I meant are you able to run the build script (assuming it works for you) and send me the dist folder (not commit it)? You could email to danny @ tuppeny .com.

If you plan to do a release soon anyway, then I'm happy to wait for that.

[fengyuanchen]

Already sent an email with released files.

[DanTup]

Seems the callback is never being fired (image.onload), but I can't figure out why. Found this suggesting it's cache related, but even appending a random number to the url doesn't seem to fix it for me. Also found this suggesting it's related to not being in the DOM.

[DanTup]

Ok, I have fixed it!

newImage = document.createElement('img');
//newImage = new Image();

Using new Image() code causes the onload callback to never fire (in our app, at least... and if I manually call it after a second the width and height properties are undefined). Using document.createElement('img') everything works fine; the callback fires; width and height are correct.

I cannot explain this difference, but I think swapping it makes sense.

[fengyuanchen]

Are you test it on a native IE8 browser?

[DanTup]

I tested both in IE8 (on Windows Vista) and in IE11 (on Windows 8.1) using the Emulation setting (I'm aware that doesn't behave exactly the same, so tend to just use it for convenience and re-test in IE8 once it was fixed in IE11's emulation mode).

In both real IE and "emulated IE" the same issue exists; we see undefined for both width and height. The change I made fixed this in both.

[fengyuanchen]

@DanTup Good, thank you, I will turn to use document.createElement('img').