Render img tag with empty src if empty string is passed to image_tag. #5020

Merged
merged 1 commit into from May 18, 2012

Projects

None yet

7 participants

@KL-7
Contributor
KL-7 commented Feb 12, 2012

Fixes #3080, but I think it might be a good idea to move that check down to AssetsPath#compute_public_path so we have same behavior for other asset tags.

Contributor
lest commented Feb 13, 2012

Personally I think it's application-specific behavior and it can be easily solved overriding image_tag in ApplicationHelper (e.g. #3080 (comment)).

Contributor
KL-7 commented Feb 13, 2012

@lest, I agree that it can be solved manually, but getting an exception from default implementation just because of an empty image source is not very pleasant. On the other hand it's harder to miss such a mistake that way.

Contributor
lest commented Feb 13, 2012

@josevalim WDYT?

Contributor
KL-7 commented Feb 13, 2012

@lest, I think no matter whether it throws an exception or renders img tag with empty src attribute it'd be better to have same behavior both in default development and production environments.

Right now with default development environment (without precompiled assets) it generates <img src="/assets"> or smth like that. The page loads without image and it's easy to miss it. On the other hand in prod environment (with precompiled assets) you get an exception that crashes the whole page while rendering it.

I agree with @KL-7, this should be the expected behaviour the img tag.

Member

This no longer merges, and will need rebased.

Contributor
KL-7 commented May 17, 2012

@steveklabnik, done.

@josevalim josevalim merged commit 6bb86e1 into rails:master May 18, 2012
Contributor

@KL-7 Hi, I found that by passing nil (instead of empty string) I also got the error in production. I am wondering if anyone can add that test and see if my finding is true, and if it is, can it be fixed too? Thanks.

Contributor
NARKOZ commented Sep 7, 2012

It still generates image tag with source to /assets/ instead of empty src attribute.

helper.image_tag ''
=> "<img alt=\"Assets\" src=\"/assets/\" />"
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment