Skip to content

image_path should return blank string if empty source is passed #7561

Closed
wants to merge 1 commit into from

4 participants

@NARKOZ
NARKOZ commented Sep 7, 2012

See #7559

@rafaelfranca
Ruby on Rails member

I think the correct implementation is right as you pointed in #7559 (https://github.com/rails/rails/blob/v3.2.8/actionpack/test/template/asset_tag_helper_test.rb#L169)

It is returning "/assets/" because you are using the assets pipeline.

@guilleiguaran could you review this one and see if it make sense?

@NARKOZ
NARKOZ commented Sep 15, 2012

@rafaelfranca shouldn't it return img src="" even with asset pipeline? There were discussions in #3080 #5020

@steveklabnik
Ruby on Rails member

As far as I'm concerned, this comment nails it. That would render invalid HTML, and adding a conditional to handle a case where you're trying to render something incorrectly sounds like a bad idea to me. 👎.

@steveklabnik
Ruby on Rails member

But then again, since we accepted #5020, apparently this ship has already sailed.

@NARKOZ
NARKOZ commented Sep 15, 2012

Yeah, I know it's invalid HTML and you can still use tag :img, src: '' (i.e. to play nice with holder.js). But I expected to see this behavior in rails 3, as #5020 was merged.

@frodsan
frodsan commented Oct 26, 2012

Any decision on this? Is it need tests?

@rafaelfranca
Ruby on Rails member

No, the implementation is right. If returns src="" when with not assets pipeline and src="assets/" with the assets pipeline.

Also I don't think we should change this since this can add incompatible changes in a stable branch.

Closing. Thank you for the pull request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.