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

Logarithmic Line Chart IE Endless Loop (2.7.1 - bundle) #5264

Closed
gnlds opened this issue Feb 12, 2018 · 5 comments
Closed

Logarithmic Line Chart IE Endless Loop (2.7.1 - bundle) #5264

gnlds opened this issue Feb 12, 2018 · 5 comments

Comments

@gnlds
Copy link

gnlds commented Feb 12, 2018

Logarithmic charts causing Endless Loop in Internet Explorer 11. You can reproduce the problem by using the link below;
https://jsfiddle.net/bj9gkn3o/4/

Expected Behavior

In Chrome the graphic works perfectly.

Current Behavior

In Internet Explorer, ChartJs causes Endless Loop while generating a set of logarithmic ticks. Then ends up with out of memory exception. The reason "significand" on ticks.

Possible Solution

if (significand === 10) {

I changed this line as "if (significand >= 10) {" and now it's working on IE 11.

Steps to Reproduce (for bugs)

https://jsfiddle.net/bj9gkn3o/4/

Context

ChartJS 2.7.1 (v2.6 also has this problem)

Environment

IE 11

@etimberg etimberg added this to the Version 2.8 milestone Feb 13, 2018
@etimberg
Copy link
Member

Thanks for reporting this. I'm surprised that this causes problems .... the variable should always be an integer since we only add 1 or set it to a rounded value.

@jcopperfield
Copy link
Contributor

jcopperfield commented Feb 13, 2018

The problem is caused by IE11 not having Math.log10 and therefore using the helpers.log10 alternate function Math.log(x) / Math.LN10;
In the given fiddle the value for tickVal is 1000, which and IE11 Math.log(1000)/Math.LN10 returns 2.9999999999999995; thus setting exp to 2 and significand to 10, instead of 3 and 1.
The significand is increased by 1 just before the check significand === 10 and will loop forever.
Maybe the function Math.log(x) / Math.LN10; should be improved to check for multiples of 10.

@etimberg
Copy link
Member

ah, ok. that makes sense. Unfortunately there is no good way to improve the polyfill on IE11. Could you PR that fix if possible?

@jcopperfield
Copy link
Contributor

I have done some testing and 1000 isn't the only whole power of 10 causing the problem. I think a possible solution could be something like the shown below. It checks if the value is actually a computed power of 10.
This should also fix the erroneous test expect(helpers.log10(1000)).toBeCloseTo(3, 1e-9); introduced in commit: 972ceb2

helpers.log10 = Math.log10 ?
  function(x) {
    return Math.log10(x);
  } :
  function(x) {
    var exponent = Math.log(x) * Math.LOG10E;
    // check for floating point rounding error
    var exponent10 = Math.floor(exponent) + 1
    var isExponent10 = (x / Math.pow(10, exponent10)) === 1;

    return isExponent10 ? exponent10 : exponent;
  };

@etimberg
Copy link
Member

Done in #5275

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

No branches or pull requests

3 participants