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

find() convert xlink:href to href #1101

Closed
exitudio opened this issue Nov 2, 2017 · 9 comments
Closed

find() convert xlink:href to href #1101

exitudio opened this issue Nov 2, 2017 · 9 comments

Comments

@exitudio
Copy link

exitudio commented Nov 2, 2017

cheerio@^1.0.0-rc.2 automatically converts xlink:href to href when using find().

cheerio.load('')('<span><svg ><use xlink:href="#1" /></svg></span>').find('use').prop('xlink:href') //undefined
cheerio.load('')('<span><svg ><use xlink:href="#1" /></svg></span>').find('use').prop('href') //#1

Since, Safari doesn't support href without the "xlink" prefix in svg yet. Maybe, leave xlink:href as it was in cheerio@^0.22.0 is better?

@peter-mouland
Copy link

some background to this issue: enzymejs/enzyme#1297 (comment)

@fb55
Copy link
Member

fb55 commented Dec 25, 2017

We're using parse5 for parsing, this seems to be an issue with its svg implementation. cc'ing @inikulin

@inikulin
Copy link
Contributor

parse5 parses xlink:href in SVG namespace as a namespaced attribute (as per spec). So, such an attribute is parsed as an attribute with the name href and xlink prefix. Cheerio uses htmlparser2 tree adapter and since original htmlparser2 hasn't concept of namespaced attributes, attribute prefixes are stored in parse5-specific x-attribsPrefix map on element:
image
(Interactive playground: https://runkit.com/inikulin/5a417be9053db70012a5954e)

It seems like Cheerio needs some additional work to support namespaced attributes in parse5 AST.

@fb55
Copy link
Member

fb55 commented Dec 25, 2017

Kk, so this is a dom-serializer shot-coming. What was the reasoning why we didn't use parse5 as a serialiser?

@kokers
Copy link

kokers commented Feb 25, 2019

Is this something that will have a fix in some near future?

@fb55
Copy link
Member

fb55 commented Dec 22, 2020

The serialization part should be fixed with the latest release.

@fb55 fb55 closed this as completed Dec 22, 2020
@ljharb
Copy link

ljharb commented Dec 22, 2020

@fb55 thanks, confirmed fixed in rc 5 in enzyme's tests!

@ljharb
Copy link

ljharb commented Dec 22, 2020

@fb55 hmm, the tests passed locally but are failing in CI in node 4: https://travis-ci.com/github/enzymejs/enzyme/builds/210326410

Specifically, parse5-htmlparser2-tree-adapter is using object destructuring syntax, which means including it is a breaking change :-/

@ljharb
Copy link

ljharb commented Dec 22, 2020

Filed #1585 for this.

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

No branches or pull requests

6 participants