search

canton7 / fuelphp-casset

Better asset management library for fuelphp (with minification!)
MIT License
103 stars 29 forks source link

Doesn't seem to handle CSS URLs when symlinked #22

Closed Daniel15 closed 12 years ago

Daniel15 commented 12 years ago

I'm encountering an issue with URLs being rewritten incorrectly in CSS files.

My setup is as follows:

The CSS files get combined correctly, but the file paths contained within are incorrect. When I use something like url(../img/background.png) in a CSS file, it comes through in the combined version as url(ublic/assets/img/background.png). This should be url(/assets/img/background.png).

canton7 commented 12 years ago

This is a known issue with the CSS URL rewriting class I'm using. Possible fixes are:

  1. Rewriting the class to rewrite to relative URLs instead, which avoids this problem
  2. Allowing the user to define their symlinks in the config or otherwise

Option 1. is nicer, but I'm afraid it will break some corner case, which makes me loathe to do it. 2. is more messy, but is a safer fix.

I'm not going to have time to tackle this for a few days, but any pull requests will be gladly accepted. If you read classes/casset/cssurirewriter.php, you'll see the rewrite method has a 4th argument, $symlinks. We just need to pass in something appropriate.

Daniel15 commented 12 years ago

In my case (and others, I'd assume), my CSS files are at /assets/css and the Casset versions are at /assets/cache/. In this case, the relative URLs are still valid (the CSS uses image paths like ../img/blah.png). So for now, maybe adding the ability to turn off CSS URL rewriting via the configuration would be good enough.

canton7 commented 12 years ago

A problem with that is that people put images which are referenced by css (backgrounds, etc) inside the css folder. These will all break if I don't do rewriting.

Right now I'm tempted to give people the option to choose their rewriter: Leave absolute as the default, add in relative (it's a really trivial piece of code), and give them the option to turn it off.

I should be able to get onto it tonight.

canton7 commented 12 years ago

Right! I'm really sorry for the delay. Exams are, however, now over.

Would you mind checking the feature/uri_rewriter branch? Readme diff (which documents the new stuff) is here. I thought combining the relative algorithm, and the option to disable rewriting, into one option was sensible, which is why I haven't merged your pull request (although thanks for submitting it!).

Thanks!

canton7 commented 12 years ago

I haven't heard anything for a while, so I went ahead and merged the branch, and pushed a new release.