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

[BUG] Browser crashes with "type: 'time'" for Y axis while hidden #5029

Closed
usethe4ce opened this issue Dec 7, 2017 · 6 comments
Closed

[BUG] Browser crashes with "type: 'time'" for Y axis while hidden #5029

usethe4ce opened this issue Dec 7, 2017 · 6 comments

Comments

@usethe4ce
Copy link

The browser freezes/crashes in this minimal test case (uncomment the indicated line to try it):

https://jsfiddle.net/aLk05ptk/10/

This is using 2.7.1.

From what testing I could do so far, the problem does not happen if the canvas is visible initially, nor when the axis type is "linear", nor when only the X axis is "time".

@jcopperfield
Copy link
Contributor

jcopperfield commented Dec 7, 2017

This bug is caused in part by the ChartJS layoutservice, which doesn't sanitize sizes to be positive. Unfortunately in the case of a time axis this negative value bubbles through buildTicks, generate, getLabelCapacity and can cause the computation of the time stepSize to become negative, which will result in an endless loop.
Changing the following line in scale.time.js in the function determineStepSize will resolve the problem.

return Math.ceil(range / ((capacity || 1) * milliseconds));

// possible fix
capacity = (capacity && capacity > 0) ? capacity : 1; // guarantee capacity is positive
return Math.ceil(range / (capacity * milliseconds));

Edit: added link to current code.

@jcopperfield
Copy link
Contributor

jcopperfield commented Dec 13, 2017

@etimberg or would it be better/preferred to make the sanity check inside the getLabelCapacity method?

return Math.floor(innerWidth / tickLabelWidth);

var capacity = Math.floor(innerWidth / tickLabelWidth);
return (capacity && capacity > 0) ? capacity : 1;

Edit:
Or should the computed stepSize in the generate method be sanitized?

if (!stepSize) {
stepSize = determineStepSize(min, max, minor, capacity);
}

if (stepSize <= 0) {
    stepSize = 1;
}

@etimberg
Copy link
Member

@jcopperfield I wonder how hard it is to sanitize the size to not be negative in the layout service. That would prevent other potential issues in the future

@jcopperfield
Copy link
Contributor

@etimberg I agree that having the layout service take care of this would be best, however it seems that the generate is called from buildTicks which is called in the update process. I think that getLabelCapacity should always return a positive capacity. This would solve the problem and would also make the line capacity || 1 in determineStepSize unnecessary.

@jcopperfield
Copy link
Contributor

Another possibility would be to skip the tick creation loop if the stepSize is invalid as it would also get rid of the dead-loop due to user misconfiguration.

for (; time < last; time.add(stepSize, minor)) {
ticks.push(+time);
}

if (stepSize > 0) {
    for (; time < last; time.add(stepSize, minor)) {
        ticks.push(+time);
    }
}

@etimberg
Copy link
Member

I agree that the label capacity should always be positive so I would recommend that as the solution for the time scale

jcopperfield added a commit to jcopperfield/Chart.js that referenced this issue Dec 14, 2017
 - infinite loop in generating time axis, due to insufficient bounds checking.
etimberg pushed a commit that referenced this issue Dec 14, 2017
- infinite loop in generating time axis, due to insufficient bounds checking.
yofreke pushed a commit to yofreke/Chart.js that referenced this issue Dec 30, 2017
- infinite loop in generating time axis, due to insufficient bounds checking.
exwm pushed a commit to exwm/Chart.js that referenced this issue Apr 30, 2021
- infinite loop in generating time axis, due to insufficient bounds checking.
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

4 participants