documentcloud / jammit

Industrial Strength Asset Packaging for Rails
http://documentcloud.github.com/jammit/
MIT License
1.16k stars 197 forks source link

incorrect image paths in packaged CSS #25

Closed ghazel closed 14 years ago

ghazel commented 14 years ago

I have a CSS file in S:/public/stylesheets/kawaii/datatable.css Inside that file are references to url(sprite.png) which exists at S:/public/stylesheets/kawaii/sprite.png

However after using Jammit to package that CSS file with a few others in the same directory, I get http://localhost:3000/assets/kawaii.css which contains incorrect paths in the CSS like this: url(../S:/public/stylesheets/kawaii/sprite.png)

I know putting images inside the stylesheets directory is probably a bad idea, but I try to keep from editing the kawaii project ( http://code.google.com/p/kawaii/ ) files.

ghazel commented 14 years ago

I see a similar problem from S:/public/stylesheets/message_block.css which says url(../../images/message_block/info_m.gif) which Jammit expands to url(../S:/images/message_block/info_m.gif)

documentcloud commented 14 years ago

Ahh, Windows absolute paths. Since Jammit is merging different stylesheets, with different relative paths, it has to rewrite relative paths to correct for the new location of the final cached package. The relevant methods are rewrite_asset_path, absolute_path, and relative_path within the Jammit::Compressor.

We're using the standard library's Pathname class to deal with these paths. Unfortunately, it doesn't look like it plays nice with Windows. This is the immediate cause of the error you're seeing:

Pathname.new("S:/public/stylesheets/message_block.css").absolute?
=> false

Whoops. I can add a quick regex test that'll match Windows drives like the above, but before we do that, I was wondering if you perhaps had a more robust solution.

ghazel commented 14 years ago

irb(main):008:0> Pathname.new("S:/public/stylesheets/message_block.css").absolute? => true

jashkenas commented 14 years ago

If the behavior is different on different platforms, then are you bundling on a non-windows box, and deploying to windows?

jashkenas commented 14 years ago

Thinking about this a little more -- I'm confused. Stylesheet urls should either be absolute to the public directory, with a leading / (not to the drive letter), or relative, in which case Jammit will try to rewrite them. Is this some kind of funky local setup where you're serving images directly off the drive, and not through a webserver? And you're compressing assets with Jammit for a purely local app?

ghazel commented 14 years ago

This is bundling on a Windows platform for a Windows platform (my dev environment). Both paths in the CSS, url(sprite.png) and url(../../images/message_block/info_m.gif), are relative. Jammit seems to be rewriting those paths to include my drive letter.

Ah, I think I found it. Seems to be quite simple:

>> ASSET_ROOT            = File.expand_path(defined?(Rails) ? Rails.root : ".") unless defined?(ASSET_ROOT)
=> "S:/"
>> PUBLIC_ROOT           = "#{ASSET_ROOT}/public"
=> "S://public"

So later when relative_path() is trying to remove S:/public, it uses S://public instead, which does not match:

>> absolute_path
=> #<Pathname:S:/public/stylesheets/kawaii/sprite.png>
>> File.join('../', absolute_path.sub(PUBLIC_ROOT, ''))
=> "../S:/public/stylesheets/kawaii/sprite.png"
documentcloud commented 14 years ago

Is Rails.root indeed "S:/" ? Or are you using an older version of Rails that relies on the RAILS_ROOT constant?

ghazel commented 14 years ago
>> RAILS_ROOT
=> "S:/"
>> Rails.root
=> #<Pathname:S:/>
>> Rails.version
=> "2.3.5"
documentcloud commented 14 years ago

Thanks. I just committed a change to use File.join instead of string interpolation, and made the PATH_TO_URL regex a little more forgiving. Give it a try and let me know if it works for you. To build and install a custom gem, just gem build jammit.gemspec, and then install the resulting gem.

ghazel commented 14 years ago

Works! Thanks.

documentcloud commented 14 years ago

Wonderful. Thanks for being willing to take the time with Windows compatibility -- I'm sure you're saving plenty of folks headaches down the line.

I've pushed out a Jammit 0.4.3 gem with the fix. Closing this ticket.