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

Exception thrown when canvas is inside a tag #5826

Closed
karaxuna opened this issue Nov 12, 2018 · 12 comments
Closed

Exception thrown when canvas is inside a tag #5826

karaxuna opened this issue Nov 12, 2018 · 12 comments
Milestone

Comments

@karaxuna
Copy link
Contributor

Expected Behavior

When target element for Chart is inside anchor tag, chart should render normally.

Current Behavior

Currently this method fails, because parent anchor tag has property host which is localhost:3000 in my case, so this method returns localhost:3000 instead of parent element.

helpers._getParentNode = function(domNode) { var parent = domNode.parentNode; if (parent && parent.host) { parent = parent.host; } return parent; };

Context

Environment

  • Chart.js version: 2.7.3
  • Browser name and version: Chrome Version 70.0.3538.77 (Official Build) (64-bit)
@simonbrunel
Copy link
Member

@karaxuna can you please share a jsfiddle that reproduces your issue?

@karaxuna
Copy link
Contributor Author

@simonbrunel
Copy link
Member

This issue comes from #5585 which tests element.host to detect if the parent node is a shadow root but <a> also exposes an host property. I should have reviewed this PR better because checking element.host is definitely not a good approach since it can match more than a shadow root.

@reda-alaoui any idea how to fix that issue?

@karaxuna as a workaround, you can wrap your canvas under a <div> element (fiddle).

@karaxuna
Copy link
Contributor Author

@simonbrunel maybe you could use this method to check if node is shadow root:
https://stackoverflow.com/a/27478691/1265753

@simonbrunel
Copy link
Member

Looks good indeed, would you be able to submit a PR which replaces if (parent && parent.host) { by if (parent && parent.toString() === "[object ShadowRoot]") and see if unit tests still pass?

@karaxuna
Copy link
Contributor Author

@simonbrunel ok

@karaxuna
Copy link
Contributor Author

@simonbrunel modified it a little bit.

@simonbrunel simonbrunel added this to the Version 2.8 milestone Nov 12, 2018
@reda-alaoui
Copy link
Contributor

@simonbrunel sorry for that :/
@karaxuna fix seems also good to me.

@simonbrunel
Copy link
Member

@reda-alaoui no worries :) did you try the fix, can you confirm it works in your case (I just approved the PR)?

@karaxuna
Copy link
Contributor Author

@reda-alaoui
Copy link
Contributor

@simonbrunel I can't test it right now.
But my automatic test should cover it, isn't it? :)

@simonbrunel
Copy link
Member

Ok, thank you guys (@karaxuna I can't open the jsfilddle).

Just want a second review then we merge the PR.

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