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.

Sign up for GitHub

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

Jump to bottom

Double quotes within single quoted attributes cause invalid output #275

Closed
davidmurdoch opened this issue Jan 2, 2018 · 5 comments
Closed

Double quotes within single quoted attributes cause invalid output #275

davidmurdoch opened this issue Jan 2, 2018 · 5 comments

Comments

@davidmurdoch
Copy link

Swapping ' for " in something like <a title='My Title Here'></a> isn't usually a problem, however, it is valid HTML to have something like <a title='Read more "Double Quotes around me"'></a>.

Critical does something strange in these cases (even with minify: false). It'll take the above HTML and output:

<a title="Read more " double="" quotes="" around="" me=""=""></a>

Which obviously isn't right.

Additionally, and likely related, if the input HTML is:

<a href=/link></a> I expect the HTML output to be exactly <a href=/link></a>. critical is currently adding in double quotes around all attributes values, i.e., <a href="/link"></a>.

My options are: {inline: true, minify: false}

Let me know if you need anything else.
@brezanac
Copy link

brezanac commented Jan 9, 2018 •
edited
Loading

I can confirm. It's especially problematic with data attributes containing values like JSON etc.

<div data-something='{ "key" : value }'></div>

becomes

<div data-something="{ " key"="" :="" value="" }"=""></div>

@bezoerb
Copy link
Collaborator

bezoerb commented Jan 23, 2018

This behavior is caused by cheerio#720 which is used under the hood by inline-critical to inline the critical css.

@sergeiliski
Copy link

This is still open and I was wondering if there was a solution to this? As far as I know, it's not possible to pass the decodeEntities: false that is suggested in the cheerio#720

@bezoerb
Copy link
Collaborator

bezoerb commented Jan 8, 2019

@sergeiliski: Can you check if your issue still exists with the prerelease version of critical npm i critical@next?

We have replaced cheerio with jsdom in inline-critical and we're only using it for the <head> section of the html. So as long as you only have double quotes within single quotes in your <body> everything should be fine.

@nairod
Copy link

nairod commented Feb 24, 2019

I faced this problem too, with the stable version. With the next version this problem has gone.
Thanks for your work!

@bezoerb bezoerb closed this as completed Mar 23, 2019
@snyk-bot snyk-bot mentioned this issue Mar 25, 2021
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@davidmurdoch @brezanac @bezoerb @sergeiliski @nairod