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

added underscore to request.svgTag regex #18

Closed
wants to merge 4 commits into from
Closed

added underscore to request.svgTag regex #18

wants to merge 4 commits into from

Conversation

LordPachelbel
Copy link

@LordPachelbel LordPachelbel commented May 21, 2018

If an SVG file has an underscore character anywhere in its <svg> tag — e.g. id="Layer_1" — then one of the regexes in this plugin's $.ajax() callback returns false, and that prevents the SVG file from being inlined.

So I added _ to that regex.

@coveralls
Copy link

Coverage Status

Coverage remained the same at 63.158% when pulling e5789fe on BrownBoots:master into 14687d2 on tiagoporto:master.

@LordPachelbel
Copy link
Author

My change did some strange things, though. (I think so, anyway.) So I'm cancelling this.

@tiagoporto
Copy link
Owner

@LordPachelbel Could you tell me what happened? Maybe I can think in another solution.

@LordPachelbel
Copy link
Author

When svgToInline() was called the resulting HTML would be a messed-up <svg> tag. I see now that my later commits put the _ in the wrong part of the regex, and it's possible our npm-grunt-bower build process ended up using the broken regex code. (I made those other commits because I didn't want to rebuild the project.)

Original regex:
request.svgTag = request.element.match(/<svg[\w\s\t\n:="\\'\/.#-]+>/g);

What I intended to do, and did correctly in the initial patch branch:
request.svgTag = request.element.match(/<svg[\w\s\t\n:="\\'\/.#-_]+>/g);

What I accidentally did in the other commits:
request.svgTag = request.element.match(/<svg[\w\s\t\n:="\\'\/.#-]+_>/g);

So maybe my original patch actually works fine.

@LordPachelbel
Copy link
Author

(Sorry that I deleted the repo with my code in 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

3 participants