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 Interchange when used on images path containing parenthesis #10264

Conversation

yannick-milanetto
Copy link
Contributor

Hi,

we faced an issue with Interchange plugin when the Interchange data contain some images with parenthesis in their URL.

The issue happens only for background style replacement.
It comes from the fact that Interchange concatenates the image path with 'url( )' which does already contain some parenthesis:
https://github.com/zurb/foundation-sites/blob/develop/js/foundation.interchange.js#L160

To fix that, i suggest escaping the parenthesis in the image path.

Example of use case
We do use Thumbor to manipulate our images.
As shown below, Thumbor's way of applying on-the-fly filter to images is by adding some keywords containing parenthesis on the picture url called:
http://thumbor.readthedocs.io/en/latest/strip_icc.html

Note for the reviewers
I've updated the test case to reflect this fix biut i'm not sure whether you prefer having a dedicated test for this case or simply, as I did, having the data used in all Interchange tests using URL containing parenthesis (this ensures that all scenarii work fine with parenthesis). Please let me know if you do prefer a dedicated test.

Cheers

@kball
Copy link
Contributor

kball commented Jun 28, 2017

Looks good to me.

@kball kball merged commit 827e781 into foundation:develop Jun 28, 2017
kball added a commit that referenced this pull request Jun 28, 2017
…esis_interchange

Fix Interchange when used on images path containing parenthesis
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.

2 participants