remy / inliner

Node utility to inline images, CSS and JavaScript for a web page - useful for mobile sites
MIT License
1.1k stars 165 forks source link

Problem with url() in inline style #152

Closed twitwi closed 7 years ago

twitwi commented 7 years ago

Thanks for this project.

When inlining an html file that contain an inline style with an url(...) inside it, the inliner seem to generate incorrect quotes.

In other words, <div style="background-image: url(im.jpg)"></div> produces something like <div style="background-image: url("data:...")" ></div> which incorrectly nests the double quotes [1].

I started making a fixture but realized there is already such a test [3] but the expected result [2] contains the quote nesting problem. I don't know the codebase enough to propose a meaningful fix, sorry.

While not being sure, a solution might be to not put any quotes around data URI. I was wondering if there could be spaces in a data URI, and found that html (only html) strips spaces in data URI [4] so they are probably never appear as part of the actual data.

Cheers,

[1] according to the w3c validator when copy/pasting [2] (as it is not hosted as html on github)

[2] https://raw.githubusercontent.com/remy/inliner/master/test/fixtures/image-style-attr.result.html

[3] https://github.com/remy/inliner/blob/master/test/fixtures/image-style-attr.src.html

[4] https://en.wikipedia.org/wiki/Data_URI_scheme#Examples_of_usage

twitwi commented 7 years ago

It seems that my solution of not quoting is probably only meaningful for base64 encoded data URI. https://drafts.csswg.org/css-values-3/#funcdef-url

So the quotes should probably be kept, and thus the fix is trickier than I expected.

twitwi commented 7 years ago

Ok. I have a patch that:

Given that all url() output change, there are 9 tests for which I need to update the output. Until I find more time to check the tests, it seems the hook won't let me push.

remy commented 7 years ago

The hooks just ensure your commit messages are in the right structure (to allow for a release). If you can rebase the commit messages then you should be good.

On Fri, 25 Aug 2017, 00:15 Rémi Emonet notifications@github.com wrote:

Ok. I have a patch that:

  • uses single quotes to avoid the nesting problem
  • tries to do the proper escapes to handle cases both where url() is in