Skip to content

Infinite loop when width or height < padding. - #5048

Closed
fanthos wants to merge 5 commits into
chartjs:masterfrom
fanthos:master
Closed

fanthos wants to merge 5 commits into
chartjs:masterfrom
fanthos:master

Conversation

@fanthos

@fanthos fanthos commented Dec 15, 2017

Copy link
Copy Markdown
Contributor

Simple fix #5047

@@ -172,6 +172,12 @@ module.exports = function(Chart) {
// Step 1
var chartWidth = width - leftPadding - rightPadding;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

A simpler way to do this more inline with existing conventions would be:

var chartWidth = Math.min(width - leftPadding - rightPadding, 0);

@fanthos

fanthos commented Dec 27, 2017

Copy link
Copy Markdown
Contributor Author

Not sure this fixed by #5041 or not.

@jcopperfield

Copy link
Copy Markdown
Contributor

@benmccann This particular case has already been addressed. That is not to say that the layoutservice is perfect. It should probably sanitize all box and areas widths/heights to be 0 or greater. However that would require more than only checking the chartarea.

@benmccann

Copy link
Copy Markdown
Contributor

@fanthos this PR has a merge conflict and will need to be rebased

@jcopperfield could you clarify where this case is being addressed?

@jcopperfield

Copy link
Copy Markdown
Contributor

@benmccann The PR #5041 fixes the infinite loop in the time scale, by having getLabelCapacity always returning a value greater or equal to zero.
Like I said before, IMO the layout service should better check all sizes to be correct, i.e. all boxes, not just the chart width/height like proposed in this PR, so that's why I think this PR is not the correct way to solve this.

@benmccann

Copy link
Copy Markdown
Contributor

@fanthos Thanks for this effort. I'm going to close this PR since it cannot be merged due to the merge conflict and it seems the issue has already been addressed. Let us know if you still have the issue after 2.7.2 is released and we still need to fix it

@benmccann benmccann closed this Jan 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG]Infinite loop when canvas width = 0 and padding > 0

3 participants