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.

Sign up for GitHub

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

Jump to bottom

asset_path respects SCRIPT_NAME. #9646

Closed
wants to merge 2 commits into from
Closed

asset_path respects SCRIPT_NAME. #9646

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a8aee4d
remove trailing whitespace
senny Mar 10, 2013
83613ca
`asset_path` respects SCRIPT_NAME.
senny Mar 10, 2013
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 2 additions & 0 deletions actionpack/CHANGELOG.md
View file
Open in desktop
@@ -1,5 +1,7 @@
## Rails 4.0.0 (unreleased) ## ## Rails 4.0.0 (unreleased) ##


* `asset_path` respects SCRIPT_NAME. Fixes #2992. *Yves Senn*

* Fix incorrectly appended square brackets to a multiple select box * Fix incorrectly appended square brackets to a multiple select box
if an explicit name has been given and it already ends with "[]" if an explicit name has been given and it already ends with "[]"


Expand Down
2 changes: 2 additions & 0 deletions actionpack/lib/action_view/helpers/asset_url_helper.rb
View file
Open in desktop
Expand Up @@ -133,6 +133,8 @@ def asset_path(source, options = {})
end end


relative_url_root = defined?(config.relative_url_root) && config.relative_url_root relative_url_root = defined?(config.relative_url_root) && config.relative_url_root
request = self.request if respond_to?(:request)
relative_url_root ||= request.script_name if request
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if respond_to?(:request) should be enough?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw this pattern down below: https://github.com/senny/rails/blob/83613ca60b1d536003a58839cd878a0fd8673be3/actionpack/lib/action_view/helpers/asset_url_helper.rb#L195

I'm not sure the request method can actually return nil

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it returns nil for ActionMailer, for example.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess we should keep it the way it is then. If this PR makes it in, I'd like to perform a quick cleanup/refactoring on the that helper code.

if relative_url_root if relative_url_root
source = "#{relative_url_root}#{source}" unless source.starts_with?("#{relative_url_root}/") source = "#{relative_url_root}#{source}" unless source.starts_with?("#{relative_url_root}/")
end end
Expand Down
14 changes: 12 additions & 2 deletions actionpack/test/template/asset_tag_helper_test.rb
View file
Open in desktop
Expand Up @@ -443,8 +443,8 @@ def test_image_alt
[nil, '/', '/foo/bar/', 'foo/bar/'].each do |prefix| [nil, '/', '/foo/bar/', 'foo/bar/'].each do |prefix|
assert_equal 'Rails', image_alt("#{prefix}rails.png") assert_equal 'Rails', image_alt("#{prefix}rails.png")
assert_equal 'Rails', image_alt("#{prefix}rails-9c0a079bdd7701d7e729bd956823d153.png") assert_equal 'Rails', image_alt("#{prefix}rails-9c0a079bdd7701d7e729bd956823d153.png")
assert_equal 'Long file name with hyphens', image_alt("#{prefix}long-file-name-with-hyphens.png") assert_equal 'Long file name with hyphens', image_alt("#{prefix}long-file-name-with-hyphens.png")
assert_equal 'Long file name with underscores', image_alt("#{prefix}long_file_name_with_underscores.png") assert_equal 'Long file name with underscores', image_alt("#{prefix}long_file_name_with_underscores.png")
end end
end end


Expand Down Expand Up @@ -733,6 +733,16 @@ def request
assert_equal "http://www.example.com/foo", @module.asset_url("foo") assert_equal "http://www.example.com/foo", @module.asset_url("foo")
end end


def test_asset_url_with_script_name
@module.instance_eval do
def request
Struct.new(:base_url, :script_name).new("http://www.example.com", "/app")
end
end

assert_equal "http://www.example.com/app/foo", @module.asset_url("foo")
end

def test_asset_url_with_config_asset_host def test_asset_url_with_config_asset_host
@module.instance_eval do @module.instance_eval do
def config def config
Expand Down