fengyuanchen / cropper

⚠️ [Deprecated] No longer maintained, please use https://github.com/fengyuanchen/jquery-cropper
MIT License
7.76k stars 1.74k forks source link

zoomDelta misbehaving in FF #128

Closed MonteShaffer closed 9 years ago

MonteShaffer commented 9 years ago

It appears on FF, the zoomDelta is 0.1 not 10%

behaves as expected in IE, Chrome

Ideally, it would be nice to set the zoomDelta as a potential option ... I don't know what the browser deltas are doing ...

wheel: function (event) { var e = event.originalEvent, msDeltaY = 117.25, // IE mozDelatY = 5, // Firefox webkitDelatY = 166.66665649414062, // Chrome, Opera zoomDelta = 0.1, // 10% delta;

  if (this.disabled) {
    return;
  }
fengyuanchen commented 9 years ago

What did you mean with this:

the zoomDelta is 0.1 not 10%

And what's the version of your Firefox?

MonteShaffer commented 9 years ago

FF 33.1 on Windows 7

I mean, with the other browsers, it behaves as expected. You use the wheel, and it zooms nicely.

With FF, it zooms very little, I have to scroll 100 times for it to zoom as much as one time on IE/Chrome

I am using your latest version here:

http://mshaffer.dev2.patent-rank.com/cropper/demos/fancy.html#top

I just tried it on FF for Ubuntu (Virtualbox), with FF 33.0 and it behaves the same way.

On WinXP, VirtualBox, the page crashed on IE8, but basically behaves as expected after the crash in regards to the ZOOM

CHROME seems to zoom much faster than the others, not the 10%

Anyway, your source refers to different browser deltas, which seems a bit odd. I attached some screen shots.

One Wheel means I used the "third-mouse" for one zoom movement. Ten Wheel means I used it 10 times.

Ten Wheel WIN 7 IE

tenwheel-win7-ie

One Wheel WinXP Chrome

onewheel-winxp-chrome

One Wheel Ubuntu FF

onewheel-ubuntu-ff

Your jquery class is well designed, and well written. There are other jquery classes that do similar things, but not as well. However, their zoom logic seems to work a bit better.

Firefox apparently does have some third-mouse wheel issues: https://support.mozilla.org/en-US/questions/987634

Your code

wheel: function (event) {
      var e = event.originalEvent,
          msDeltaY = 117.25, // IE
          mozDelatY = 5, // Firefox
          webkitDelatY = 166.66665649414062, // Chrome, Opera
          zoomDelta = 0.1, // 10%
          delta;

These numbers with modulus seem a bit arbitrary ... and not very stable for versions within a browser version.

MonteShaffer commented 9 years ago

https://github.com/jackmoore/wheelzoom/blob/master/wheelzoom.js

function onwheel(e) {
            var deltaY = 0;

            e.preventDefault();

            if (e.deltaY) { // FireFox 17+ (IE9+, Chrome 31+?)
                deltaY = e.deltaY;
            } else if (e.wheelDelta) {
                deltaY = -e.wheelDelta;
            }
fengyuanchen commented 9 years ago

I tested the wheel zoom in IE8/9/10/11, Firefox 33/34, Chrome 37/38 on Window 7, and it worked fine. But I didn't test it on Ubuntu or other platforms.

MonteShaffer commented 9 years ago

Here is my code that works as expected ... I anchor to FF as the default zoom factor

Minimal browser detection

/* http://stackoverflow.com/questions/12625876/how-to-detect-chrome-and-safari-browser-webkit */
var isOpera = !!window.opera || navigator.userAgent.indexOf(' OPR/') >= 0, // Opera 8.0+ (UA detection to detect Blink/v8-powered Opera) 
    isFirefox = typeof InstallTrigger !== 'undefined', // Firefox 1.0+ 
    isSafari = Object.prototype.toString.call(window.HTMLElement).indexOf('Constructor') > 0, // At least Safari 3+: "[object HTMLElementConstructor]" 
    isChrome = !!window.chrome && !isOpera, // Chrome 1+ 
    isIE = /*@cc_on!@*/ false || document.documentMode; // At least IE

wheel function

wheel: function (event) {
      var e = event.originalEvent,
            browserFactor = 1,
            deltaY = 0;

      if (this.disabled) {
        return;
      }

      if(isChrome) { browserFactor = 1/33;}
      if(isIE) { browserFactor = 1/25;}

      event.preventDefault();

       if (e.deltaY) { // FireFox 17+ (IE9+, Chrome 31+?)
                deltaY = e.deltaY;
            } else if (e.wheelDelta) {
                deltaY = -e.wheelDelta;
            }

      this.zoom(deltaY * this.defaults.zoomDelta * browserFactor);
    },

your "data" object should probably capture the current "zoomLevel" and "aspectRatio"

e.g., .done(

fengyuanchen commented 9 years ago

I had improved this, you may try the v0.7.8 now.