ActionView::Helpers::AssetUrlHelper#asset_path uses request.script_name in an engine

[jnbt]

I encountered the following problem with current master branch (4.0.0.beta):

The asset_path method seems to use the request.script_name information to determine the correct url for the assets.
But when you try to include assets per assets_path in an engine (or a depending method e.g. stylesheet_link_tag) than this script_name is prefixed to the urls.

Steps to reproduce:

1. Generate a fresh rails 4.0.0.beta project
2. Generate a fresh isolated mountable engine (like the example in the edge guide) with a dummy controller and a dummy method foo#bar which uses a layout.
3. Require the engine as a gem in the rails Gemfile
4. Mount the engine, e.g. mount Blorgh::Engine, :at => 'blorgh'
5. Start the server

Problem: The stylesheet-link url is generates

 stylesheet_link_tag("blorgh/application") => "/blorgh/assets/blorgh/application.css?body=1"

I temporarily fix this problem by setting config.relative_url_root = '' in application.rb of the main project.

[steveklabnik]

I swear I saw another ticket about this, but don't remember which one.

/cc @drogus @josh

[josh]

Code is here https://github.com/rails/rails/blob/master/actionpack/lib/action_view/helpers/asset_url_helper.rb#L135-139

I never understood the point of adding the script name. I'd rather just get rid of it and make engines responsible for namespacing their assets if thats what they actually want to do.

[drogus]

I will need to look at the history of this file, I think that I originally used SCRIPT_NAME before assets pipeline, but this part was added later.

[trevorturk]

I ran into this problem today. I don't think an engine can use stylesheet_link_tag to link to its own assets.

+1 to what @josh said unless @drogus can think of a good reason to leave things like this and knows of a workaround?

[chrismo]

Also ran into this problem today. Commit 8e26d29 (#910 / #5296) seems to have introduced the :script_name check. The tests that were added with 8e26d29 were removed with 607829a. If I remove line 136, all current tests in actionpack still pass - which might indicate some test coverage was lost, but I didn't dig through the refactoring work around 607829a to know for certain.

5b3bb61 may be related - dunno - all around Rack's SCRIPT_NAME env var. Perhaps the scenario outlined in #910 is slightly at odds with the Rails engine scenario -- or perhaps asset pipeline needs to be able to take these cases into account. My head is too dull right now to connect any dots - but if any of this helps, figured I'd put it here.

[rafaelfranca]

@guilleiguaran could you take a look at this one?

[drogus]

Sorry for being quiet for so long, I agree with others here (script_name should not be taken into account here), you can read explanation on why this thing was there in the PR #9021

[matthijskooijman]

I'm having a bit of trouble with the fix for this issue. I'm using a plain rails app, with no engines mounted on subdirectories (at least I think so, I'm not 100% sure about terminology here). However, the entire application is mounted on a subdirectory, which nginx correctly passes to Rails in the SCRIPT_NAME variable.

For regular links this works as expected, the SCRIPT_NAME gets prepended to all links within the application. However, for assets, this does not work, it generates /assets/foo instead of /subdir/assets/foo. If I revert the commit referenced above, everything works again as expected.

I realize I can fix this by setting config.relative_url_root, but that defeats the point of SCRIPT_NAME and requires me to actually configure the subdir in two places (nginx and rails) even though that shouldn't be needed.

As far as I understand things, the original problem was that, inside a mounted engine, request.script_name would include the mount point, resulting in incorrect urls for assets inside such a mounted engine. In other words, inside a mounted engine, request.script_name would be incorrect for generating assets urls. Therefore, the above commit stops using request.script_name altogether.

AFAICS, a better fix would be to always use the script_name originally passed by the webserver and Rack, even in a mounted engine. In that way, all assets would use the original script_name and get the correct url. I'm fairly new to Rails so I can't really comment on how to achieve this, though.

Does this make sense?

[pixeltrix]

@matthijskooijman the problem is that SCRIPT_NAME isn't available during the asset compilation task so config.relative_url_root needs to be set so that it knows where they will be served from by the webserver. This is especially important for Sass since the asset helpers like image-url, etc. will generate absolute urls.

[matthijskooijman]

Ah, you're saying that the asset compilation happens on startup instead of on-demand during the first request, so you don't know what SCRIPT_NAME will be?

[pixeltrix]

No, asset compilation (for production) either happens as part of the deployment process or offline via rake assets:precompile and checked into version control. For dynamic compilation during development each of the asset paths registered (app/assets, vendor/assets, engine/app/assets, etc.) are searched for the asset requested. It's up to the engine developer if they want to namespace their assets, e.g. /assets/blog/path/to/image.png would be blog_engine/app/assets/images/blog/path/to/image.png.

Basically, assets are under a single global namespace - typically /assets but can be configured to something else. The code that was removed assumed individual namespaces for each engine, e.g. /assets and /blog/assets which was the original method of using static files from individual engines.