cheton / browserify-css

A Browserify transform for bundling, rebasing, inlining, and minifying CSS files.
http://cheton.github.io/browserify-css/
MIT License
144 stars 22 forks source link

Does not correctly join paths on Windows #27

Closed bricketts-authentify closed 8 years ago

bricketts-authentify commented 8 years ago

When running Node/Browserify on Windows, CSS paths (such as in url() values) do not get formatted correctly. path.join formats paths with a backslash (\) on Windows, which does not get escaped when written to the CSS, causing the browser to request an invalid path.

Example:

@font-face {
    ...
    src: url("../fonts/material-icons.ttf");
    ...
}

in the source CSS file gets turned into

@font-face {
    ...
    src: url(fonts\material-icons.ttf);
    ...
}

in the transformed CSS file, which Firefox requests as

~/fontsmaterial-icons.ttf

which obviously is not correct.

One solution would be to use the path-posix module to always use forward slashes (/) even when on Windows.

cheton commented 8 years ago

I guess it may result from the following line: https://github.com/cheton/browserify-css/blob/master/css-transform.js#L96

newUrl = processRelativeUrl(path.relative(from, to));

The path.relative(from, to) will return a relative path with platform-specific path separator.

Since I don't have a Windows OS installed for verification, could you try to implement the processRelativeUrl callback to convert \ to / as a temporary workaround? For example:

var path = require('path');
var browserify = require('browserify');

browserify(options)
    .add('./index.js')
    .transform(require('browserify-css'), {
        processRelativeUrl: function(relativeUrl) {
            return relativeUrl.replace(/\\/g, '/');
        }
    })
    .bundle();

I will fix the path of the returned newUrl from processRelativePath if above example would work for you!

cheton commented 8 years ago

I just ran it on Windows 10, all backslashes (\) can be successfully converted to forward slashes (/) using above example:

processRelativeUrl: function(relativeUrl) {
    return relativeUrl.replace(/\\/g, '/');
}

May I know if above example may solve your problem?

cheton commented 8 years ago

I made the bug fixes in v0.8.4. Please let me know if you have any further questions. Thanks.

bricketts-authentify commented 8 years ago

v0.8.4 has fixed the issue without me needing to do any path processing. Thanks!