/core

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 #221 - Image src URLs with spaces in them don't work for fittedImage and croppedImage #222

Closed
wants to merge 2 commits into
base: master
from

Conversation

Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@srikumarks @evancz
@srikumarks

srikumarks commented Apr 21, 2015

The problem was that the native implementations of fittedImage and
croppedImage used CSS background-image, but were not encoding the
URLs before generating the property. This commit encodes the src
parameter using encodeURIComponent, which makes them work correctly.
@srikumarks

This comment has been minimized.

Show comment
Hide comment
@srikumarks

srikumarks Apr 21, 2015

The best way to deal with such string safety is via the type system. A URL type should be used instead of String for these image source arguments.

srikumarks commented Apr 21, 2015

The best way to deal with such string safety is via the type system. A URL type should be used instead of String for these image source arguments.
@srikumarks

This comment has been minimized.

Show comment
Hide comment
@srikumarks

srikumarks Apr 23, 2015

The Travis failure, btw, is a failure to install elm-compiler-0.15.

srikumarks commented Apr 23, 2015

The Travis failure, btw, is a failure to install elm-compiler-0.15.

srikumarks added some commits Apr 26, 2015

@srikumarks
For #221 - Added encodeURI and family of functions to Basics so that 
they can be used to make hrefs correctly in Graphics.Element.
2ad079b
@srikumarks
For #221 - The src parameter needs to appear within quotes to work co… 
…rrectly in some cases.

For valid URLs with parentheses in them, for example, the fittedImage and tiledImage
functions won't set the correct style. This quotes the result, assuming that src is a
valid URL encoded string. Javascript's encodeURI function escapes double quotes and
can be used to convert things like file paths to href values.
b8a94d0
@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz Jun 30, 2015

Member

Can you show an example of a "good string" that can be given to these functions that causes an unexpected result?
Member

evancz commented Jun 30, 2015

Can you show an example of a "good string" that can be given to these functions that causes an unexpected result?
@srikumarks

This comment has been minimized.

Show comment
Hide comment
@srikumarks

srikumarks Jul 2, 2015

I'd noted a few in my issue comment here. For example, passing "hello).png" would generate "url(hello).png)" for the "background" property which fails to parse.

srikumarks commented Jul 2, 2015

I'd noted a few in my issue comment here. For example, passing "hello).png" would generate "url(hello).png)" for the "background" property which fails to parse.

@jvoigtlaender jvoigtlaender referenced this pull request Jan 19, 2016

Closed

Image src URLs with spaces in them don't work for fittedImage and croppedImage #221

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz May 10, 2016

Member

I don't really understand what is going on here or what would be good, but the Graphics.* modules have moved to evancz/elm-graphics so it makes sense to retarget stuff like this.
Member

evancz commented May 10, 2016

I don't really understand what is going on here or what would be good, but the Graphics.* modules have moved to evancz/elm-graphics so it makes sense to retarget stuff like this.

@evancz evancz closed this May 10, 2016

@evancz

This comment has been minimized.

Show comment
Hide comment
@evancz

evancz May 10, 2016

Member

Oh, I get it from the last comment. The change in the one file makes sense, but the other one is weird.
Member

evancz commented May 10, 2016

Oh, I get it from the last comment. The change in the one file makes sense, but the other one is weird.

@jvoigtlaender jvoigtlaender referenced this pull request Jul 8, 2016

Open

Make fittedImage and croppedImage work for URLs with spaces #11

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment