Adding CSS `url()` rewriting

[pferreir]

Hi there!

I was having some problems with url() statements in minified/bundled CSS files containing relative URLs. So, I added URL rewriting capability to Bundle up. I've played with it a bit and it seems to work just fine. The unit tests seem to be OK too.

I've also added a generatedDir config option, just because it seemed right :) I hope it's OK with you.

Could we have this in the official release? I tried to carefully check my code since I'm quite new to CoffeeScript. So, feel free to correct it.

Thanks!

[pferreir]

There's a unit test failing, but I believe it's got nothing to do with my changes. It's failing in master too.

[Cowboy-coder]

Awesome, thanks a lot! :) I will try to get this merged in the a day or two. I will also fix the failing test. Would it be possible for you to add some unit tests for this CSS URL Rewriting thing? Not a biggie, I can probably fix it during merge-time otherwise but it helps me understand this feature as well. Is it rewriting

background: url(/images/bg.jpg)

to

/* assuming staticUrlRoot:"http://static.example.com" */
background: url(http://static.example.com/images/bg.jpg)

?

[pferreir]

No, it is rather comparing the initial directory the CSS file was in with the final "bundle" one, and prepending (and normalizing) the corresponding relative path before each url(...) resource. I will add some unit tests later today.

[Cowboy-coder]

Cool! I fixed the failing test in master btw.

[pferreir]

OK, I've made a small fix in a regular expression. I've also added two small unit tests. I hope they're OK. Please let me know if that's not the case :)

[Cowboy-coder]

Looked at this patch yesterday and it looked great. I just need to figure out why I get an 'Unexpected Indent¨-error when I try bundle-up from the current Master branch (not noticed when I run tests). Probably some changes in coffee-script that's causing the error. Will merge this pull request after I've fixed that.

[hammat]

Hi guys, thanks for your great work! Any update about merging pferreir's pull request? Cheers

[Cowboy-coder]

Sorry guys for the late response.

This pull request seems to be breaking some styles for me url(/relative/to/root.png) gets rewritten to url(../relative/to/root.png) I think it should only replace urls if the path doesn't start with a /. Also it breaks base64 encoded images.

But do we really need this? Wouldn't it be easier to just reference the images starting with a /, which would solve all these issues, because then the images would be relative to the web server root instead of relative to the stylesheet?

[danielsantiago]

@Cowboy-coder but what about some external libraries like "font-awesome" that use relative path by default to find font files. I thinks this is a must.

[pferreir]

I agree with @hlwebs, having relative URLs is kind of a must. I, for one, tend to avoid using absolute URLs in CSS files since I like my apps to be deployable everywhere, even in http://myserver/myapp/ setups.

I believe that just excluding paths starting with / would suffice. I can have a look at it later, if you agree that it's the way to go.

[danielsantiago]

@pferreir have you ever look at Assetic (asset management framework for PHP)? They have a css rewrite filter that fixes relative URLs, maybe we can follow they implementation: https://github.com/kriswallsmith/assetic/blob/master/src/Assetic/Filter/CssRewriteFilter.php

I have never done anything in CoffeeScript but I will try to help.

[Cowboy-coder]

Yeah, ok, I agree! :)

Would actually if possible be pretty cool if this was based on staticUrlRoot in some way. Like if I send staticUrlRoot:"http://cdn-example.com" all urls (relative and absolute) in the css would properly be prefixed with "http://cdn-example.com".

What do you guys think?

[pferreir]

Even URLs that start with / ? I would expect those to refer server/, regardless of what staticUrlRoot says... I think there are two different use cases here:

1. Fixing relative URLs that get broken because the final path of a bundle is different from its "development" one;
2. Replacing or re-locating relative or root (/) URLs;

I was trying to address the first case, but maybe with some extra work we can kill the two birds with one stone. For instance, webassets uses an interesting approach:

https://github.com/miracle2k/webassets/blob/master/src/webassets/filter/cssrewrite/__init__.py#L78

Basically, the filter accepts a "replace" function or a dictionary mapping paths, otherwise it falls back to doing point 1 above.

[danielsantiago]

I really like the idea of a "replace function" for a custom need.