Skip to content

Fix helper._calculatePadding return NaN causes Maximum Call Stack Size Exceeded#6857

Merged
etimberg merged 1 commit intochartjs:releasefrom
duykhang53:release
Dec 23, 2019
Merged

Fix helper._calculatePadding return NaN causes Maximum Call Stack Size Exceeded#6857
etimberg merged 1 commit intochartjs:releasefrom
duykhang53:release

Conversation

@duykhang53
Copy link

I encounter an issue that causes maximum call stack size exceeded in Chrome by using ng2-charts, found out that was in issue from this library.

The root cause from helpers.getStyle() inside helpers._calculatePadding() return an empty string, so parseInt() will result NaN that cause fitBoxes() recursive called infinitively.

@kurkle
Copy link
Member

kurkle commented Dec 22, 2019

I'm curious, when does this happen?

@duykhang53
Copy link
Author

It happens when the element is not yet attached to DOM, I think.

My Angular project place that <canvas> inside a <div> and later on it bind that <div> to DOM. It really weird that getComputedStyle() return an empty string for padding property.

@duykhang53
Copy link
Author

image

@kurkle
Copy link
Member

kurkle commented Dec 22, 2019

Another curiosity, does this change make it work alright?

@duykhang53
Copy link
Author

Yes, of course!

@kurkle
Copy link
Member

kurkle commented Dec 22, 2019

I suppose resize gets called again, when attached to DOM. What browser are you testing with and did you test any others?

@duykhang53
Copy link
Author

I tested on Chrome, Firefox and Edge, it throws an exception in zone.js (Angular mechanism), I think. That causes the code for that component no further running.

fitBoxes() was called infinitely because width and height in updateDims() compare NaN to NaN (it always false). So the issue is from resize() call in constructor.

@kurkle
Copy link
Member

kurkle commented Dec 22, 2019

#5955 is loosely related

*.swp
*.stackdump

chart.js-*.tgz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similar to the dist folder, when you run npm pack, it will generate chart.js-<version>.tgz which use for local npm install. It may use for testing when the package is not uploaded to npmjs.

@duykhang53 duykhang53 changed the title Fix helper._calculatePadding return NaN casue Maximum Call Stack Size Exceeded Fix helper._calculatePadding return NaN causes Maximum Call Stack Size Exceeded Dec 22, 2019
@etimberg etimberg merged commit 0c18c9a into chartjs:release Dec 23, 2019
@duykhang53
Copy link
Author

duykhang53 commented Dec 23, 2019

Thanks for the merge, btw can you guys update npm version tag?

https://travis-ci.org/chartjs/Chart.js/builds/628564087#L5259

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants