Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Respect absolute paths in compute_source_path. #6752

Merged
merged 1 commit into from Jun 16, 2012

Conversation

Projects
None yet
2 participants
Member

steveklabnik commented Jun 16, 2012

When using compute_source_path to determine the full path of an
asset, if our source begins with '/', we don't want to include
the directory. Examples are illustrative:

compute_source_path("foo", "stylesheets", "css")
# => "/Users/steve/src/my_app/public/stylesheets/foo.css"
compute_source_path("/foo", "stylesheets", "css")
# => "/Users/steve/src/my_app/public/foo.css"

Before this patch, the second example would return the same as the
first.

Fixes #5680.

If this is accepted, I will also backport to 3.2.

@rafaelfranca rafaelfranca commented on the diff Jun 16, 2012

actionpack/test/template/asset_tag_helper_test.rb
@@ -1298,6 +1295,17 @@ def test_caching_stylesheet_include_tag_when_caching_off
assert !File.exist?(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'money.css'))
end
+
+ def test_caching_stylesheet_include_tag_with_relative_uri
+ ENV["RAILS_ASSET_ID"] = ""
+
+ assert_dom_equal(
+ %(<link href="/stylesheets/all.css" media="screen" rel="stylesheet" />),
+ stylesheet_link_tag("/foo/baz", :cache => true)
@rafaelfranca

rafaelfranca Jun 16, 2012

Owner

I didn't get this test. If you use absolute path it will not generate the cached asset?

@steveklabnik

steveklabnik Jun 16, 2012

Member

See line 1307.

... the test may be bad, I'm not an expert at these tests. It fails without my code change and passes with it, though.

@rafaelfranca

rafaelfranca Jun 16, 2012

Owner

Reading the issue I could undertand better.

Thanks.

@rafaelfranca rafaelfranca and 1 other commented on an outdated diff Jun 16, 2012

actionpack/test/template/asset_tag_helper_test.rb
@@ -1298,6 +1295,17 @@ def test_caching_stylesheet_include_tag_when_caching_off
assert !File.exist?(File.join(ActionView::Helpers::AssetTagHelper::STYLESHEETS_DIR, 'money.css'))
end
+
+ def test_caching_stylesheet_include_tag_with_relative_uri
@rafaelfranca

rafaelfranca Jun 16, 2012

Owner

Should not be test_caching_stylesheet_include_tag_with_absolute_uri?

@steveklabnik

steveklabnik Jun 16, 2012

Member

Yes. I'll fix that and push now.

@steveklabnik steveklabnik Respect absolute paths in compute_source_path.
When using compute_source_path to determine the full path of an
asset, if our source begins with '/', we don't want to include
the directory. Examples are illustrative:

> compute_source_path("foo", "stylesheets", "css")
=> "/Users/steve/src/my_app/public/stylesheets/foo.css"
> compute_source_path("/foo", "stylesheets", "css")
=> "/Users/steve/src/my_app/public/foo.css"

Before this patch, the second example would return the same as the
first.

Fixes #5680.
afb053b

@rafaelfranca rafaelfranca added a commit that referenced this pull request Jun 16, 2012

@rafaelfranca rafaelfranca Merge pull request #6752 from steveklabnik/fix_5680
Respect absolute paths in compute_source_path.
bebfa5c

@rafaelfranca rafaelfranca merged commit bebfa5c into rails:master Jun 16, 2012

@rafaelfranca rafaelfranca added a commit that referenced this pull request Jun 16, 2012

@rafaelfranca rafaelfranca Merge pull request #6752 from steveklabnik/fix_5680
Respect absolute paths in compute_source_path.
52d0963
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment