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

Fixed SVG base64 encoded references in the css to be wrapped in singl… #97

Closed
wants to merge 1 commit into from

Conversation

tim-white87
Copy link

…e quotes.

@remy
Copy link
Owner

remy commented May 11, 2016

I can't merge this at the moment, it's failing tests (and doesn't include tests itself). The commit message also needs rebasing (which I can help with if you need).

I assume this is related to #96. If so, can you at least provide a working demo of the inliner failing - the directions here should help guide you.

Thanks so much for the PR and I'll help as much as I can to get it merged in.

@tim-white87
Copy link
Author

Hey Remy,
This is the same issue as #96. I wasn't able to create a pull request on that ticket so I created a new one. Here is an example of a base 64 encoded CSS background property generated with inliner: http://codepen.io/anon/pen/QNRjvN

Notice how the background base64 encoded SVG does not display. Here is a fork of the same code, only this time the background CSS URI is wrapped in single quotes: http://codepen.io/anon/pen/vGwNZP

The background image for this displays now. I think it has something to do with the encoding of the URIs, but I am not too familiar with why the single quotes are necessary (something not getting escaped properly perhaps?).

We are using this module for an internal tool, so any help in getting this fixed would be greatly appreciated.

Cheers!
Tim

@remy
Copy link
Owner

remy commented May 21, 2016

Hey there,

I'm trying to pick up outstanding issues on this repo today, but the codepens you sent me to, I don't know how to get the full (unencumbered) output (I'm also the author of jsbin, so there's not really much reason to learn codepen either 😄).

Can you put the raw output on a gist for me to try to replicate and merge?

@remy
Copy link
Owner

remy commented May 21, 2016

Actually, I was able to replicate in jsbin https://jsbin.com/vagitel/edit?html,css,output so I now see what's going on.

I'll probably have to create this commit anew only because the commit format is wrong, and it's failing the linting tests.

@remy remy closed this in 253a317 May 22, 2016
@remy
Copy link
Owner

remy commented May 22, 2016

Heh. The change wasn't quite as simple as your PR, but it's in now and has tests. Cheers for spotting it.

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 this pull request may close these issues.

None yet

2 participants