albertogasparin/Optiscroll

Bug inside calc() method - no scrollbars needed

henryruhs opened this issue · 2 comments

Hey,

I hacked around your code to analyse why Optiscroll generates unneeded scrollbars in Chrome.

Please read the code comments above!

Using Math.max instead Math.min fixed the issue but I think we have to check if sizeRatio is lower than maxTrackSize instead?

I did not completly understand the calc() magic, please tell me if you can confirm this bug.

Math bugfix:

sizeRatio = Math.max(sizeRatio, settings.maxTrackSize / 100);

Another bugfix:

if (sizeRatio < settings.maxTrackSize) {
    return { position: 0, size: 1, percent: 0 };
}

Related fiddle: http://jsfiddle.net/ty4ca48z/4/ (uncomment related code to see useless scrollbar)

Method with debug and comments:

calc: function () {
      var position = scrollEl[scrollProp],
          viewS = cache[clientSize], 
          scrollS = cache[scrollSize], 
          sizeRatio = viewS / scrollS,
          sizeDiff = scrollS - viewS,
          positionRatio, percent;

      if(sizeRatio === 1 || scrollS === 0) { // no scrollbars needed
        return { position: 0, size: 1, percent: 0 };
      }

      percent = 100 * position / sizeDiff;

      // prevent overscroll effetcs (negative percent) 
      // and keep 1px tolerance near the edges
      if(position <= 1) { percent = 0; }
      if(position >= sizeDiff - 1) { percent = 100; }
      
      // Capped size based on min/max track percentage 
      sizeRatio = Math.max(sizeRatio, settings.minTrackSize / 100);
      sizeRatio = Math.min(sizeRatio, settings.maxTrackSize / 100);

      positionRatio = (1 - sizeRatio) * (percent / 100);

      // positionRatio returns 0.050000000000000044
      // sizeRatio in my case returns 95 and therefore generates a useless scrollbar
      console.log(positionRatio);
      console.log(sizeRatio);
        
      return { position: positionRatio, size: sizeRatio, percent: percent };
    }

Congratulations! You just found a bug in Chrome (related to this) 😄
The problem is a rounding error in Chrome due to the CSS units you are using.

Let me explain: Optiscroll calc() assumption is that scrollHeight is always equal or bigger than scrollHeight, as the spec says (same for widths). However, vh and vw units in Chrome are rendered with floating point precision BUT they are rounded differently by Chrome internals.

So, for instance, if an element has an height = 85,672px, Chrome clientHeight is 86px but scrollHeight is 85! Definitely wrong.

Nevertheless, to fix this, it should be enough to change sizeRatio === 1 to:

if(sizeRatio >= 1 || scrollS === 0) { // no scrollbars needed
        return { position: 0, size: 1, percent: 0 };
}

So if scrollHeight / clientHeight >= 1 we assume no scrollbars are needed anyway.
I don't have time now to properly check it, but feel free to try.

It seems like this solves the issue, can you please hotfix this inside your master / develop branch so I can switch bower to this branch for now. Let me know if you had time for a patch release.

Thanks for your help