Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

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

Closed
wants to merge 1 commit into from

4 participants

@NARKOZ

See #7559

@rafaelfranca
Owner

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

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

@steveklabnik
Collaborator

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. :-1:.

@steveklabnik
Collaborator

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

@NARKOZ

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

Any decision on this? Is it need tests?

@rafaelfranca

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
This page is out of date. Refresh to see the latest.
Showing with 1 addition and 1 deletion.
  1. +1 −1  actionpack/lib/sprockets/helpers/rails_helper.rb
View
2  actionpack/lib/sprockets/helpers/rails_helper.rb
@@ -59,7 +59,7 @@ def asset_path(source, options = {})
alias_method :path_to_asset, :asset_path # aliased to avoid conflicts with an asset_path named route
def image_path(source)
- path_to_asset(source)
+ source.empty? ? '' : path_to_asset(source)
end
alias_method :path_to_image, :image_path # aliased to avoid conflicts with an image_path named route
Something went wrong with that request. Please try again.