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

Element.closest doesn't work correctly on SVG elements #40

Closed
tremby opened this issue Aug 24, 2018 · 2 comments · Fixed by #41
Closed

Element.closest doesn't work correctly on SVG elements #40

tremby opened this issue Aug 24, 2018 · 2 comments · Fixed by #41

Comments

@tremby
Copy link
Contributor

tremby commented Aug 24, 2018

Say I have a page with an inline SVG which contains a path element.

In Firefox if I do document.querySelector('path').closest('html') I'll get the HTML element.

But in IE11 with the polyfill currently in this repo, I get nothing. This is because parentElement isn't defined on SVG elements.

I notice that the polyfill in this repo doesn't match that on MDN. I guess it's been updated. The one currently on MDN has el.parentElement || el.parentNode in one spot, so I think this issue has been fixed there.

https://developer.mozilla.org/en-US/docs/Web/API/Element/closest#Polyfill

I was going to open a PR with the updated code, but noticed that this polyfill also polyfills Element.prototype.matches. With that in mind I'm not sure how the code for this repo should look. Should it import the Element.prototype.matches polyfill, and so rely on it as a dependency?

@msn0
Copy link
Owner

msn0 commented Aug 24, 2018

I think we can update closest.js to current mdn version and rely on existing Element.prototype.matches, e.g.

import closest from './closest';
import '../Element.prototype.matches';

if (window.Element && !Element.prototype.closest) {
    Element.prototype.closest = closest;
}

@msn0 msn0 closed this as completed in #41 Aug 24, 2018
msn0 pushed a commit that referenced this issue Aug 24, 2018
This patch updates the Element.prototype.closest polyfill to the latest found at
https://developer.mozilla.org/en-US/docs/Web/API/Element/closest#Polyfill

This polyfill now automatically pulls in the Element.prototype.matches polyfill
as a dependency.

Closes #40
@msn0
Copy link
Owner

msn0 commented Aug 24, 2018

  • mdn-polyfills@5.10.1

Enjoy ;)

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 a pull request may close this issue.

2 participants