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

Fix issue with prefixIDs plugin not replacing url() refs correctly #908

Merged
merged 2 commits into from
Feb 19, 2018

Conversation

harrisjose
Copy link

When using the prefixIDs plugin on an SVG with multiple linear gradients, url() values are not replaced correctly because we use exec to capture values using the regex. Exec preserves the last matched index, which is undesirable in this case.

Please ref this stackoverflow answer for more information about this.

Harris P Jose added 2 commits February 19, 2018 10:54
When using the prefixIDs plugin  on an svg with multiple linearGradients, url() values are not replaced correctly
Global regexs must be reseted after every exec call since they preserve the last matched index. Since we use `css-url-regex`, creating a new regex before `exec`ing fixes this.
@harrisjose
Copy link
Author

@strarsis @caub

Can you guys take a look at this and let me know if this is okay?

@caub caub added the bug label Feb 19, 2018
@caub caub self-requested a review February 19, 2018 09:56
@strarsis
Copy link
Contributor

strarsis commented Feb 19, 2018

@harrisjose: Good catch. There is an open issue about this in the css-url-regex module.

@harrisjose
Copy link
Author

harrisjose commented Feb 19, 2018

@strarsis I thought I'd already looked in the css-url-regex module for this. Oh well.

If there aren't any further changes required, can we get this merged and released? I'd really appreciate it cause we're running into this issue with a few svgs at work. Thanks.

@caub caub merged commit 4dbd257 into svg:master Feb 19, 2018
@harrisjose
Copy link
Author

@GreLI
Sorry if I'm being a nuisance but can we get this released as a patch? Would help a lot. Thanks.

@caub
Copy link
Contributor

caub commented Feb 26, 2018

@harrisjose done

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

Successfully merging this pull request may close these issues.

3 participants