componentjs / component

frontend package manager and build tool for modular web applications
https://github.com/componentjs/guide
MIT License
4.55k stars 306 forks source link

css build bug #550

Closed jasonkuhrt closed 10 years ago

jasonkuhrt commented 10 years ago
@font-face {
  font-family: "digital-7";
  src: url('https://s3.amazonaws.com/media.littlebits.cc/fonts/digital-7/digital-7_mono-webfont.eot');
  src: url('https://s3.amazonaws.com/media.littlebits.cc/fonts/digital-7/digital-7_mono-webfont.eot?#iefix') format('embedded-opentype'),
       url('https://s3.amazonaws.com/media.littlebits.cc/fonts/digital-7/digital-7_mono-webfont.woff') format('woff'),
       url('https://s3.amazonaws.com/media.littlebits.cc/fonts/digital-7/digital-7_mono-webfont.ttf') format('truetype'),
       url('https://s3.amazonaws.com/media.littlebits.cc/fonts/digital-7/digital-7_mono-webfont.svg#digital-7_monomono') format('svg');
  font-weight: bold;
  font-style: normal
}

Compiles to:

@font-face {
  font-family: "digital-7";
  src: url('url(https://s3.amazonaws.com/media.littlebits.cc/fonts/digital-7/digital-7_mono-webfont.eot)');
  src: url('url(https://s3.amazonaws.com/media.littlebits.cc/fonts/digital-7/digital-7_mono-webfont.eot?#iefix)') format('embedded-opentype'),
       url('url(https://s3.amazonaws.com/media.littlebits.cc/fonts/digital-7/digital-7_mono-webfont.woff)') format('woff'),
       url('url(https://s3.amazonaws.com/media.littlebits.cc/fonts/digital-7/digital-7_mono-webfont.ttf)') format('truetype'),
       url('url(https://s3.amazonaws.com/media.littlebits.cc/fonts/digital-7/digital-7_mono-webfont.svg#digital-7_monomono)') format('svg');
  font-weight: bold;
  font-style: normal;
}
⧑ component -V
1.0.0-rc5

I'm trying to figure out if this bug comes from @ai's autoprefixer or not.

jasonkuhrt commented 10 years ago

This has nothing to do with @ai's autoprefixer.

The issue is css-url-rewriter (https://github.com/callumlocke/css-url-rewriter/blob/master/lib/css-url-rewriter.js) has regexp that reads url(...) (http://refiddle.com/refiddles/match-multiple-urls-within-a-css-property-value) which component/builder2.js (https://github.com/component/builder2.js/blob/master/lib/plugins/url-rewriter.js#L27-L39) treats AS IF IT WERE just the ... part (from above), see:

[...]
      file.string = rewriteCSSURLs(string, function (uri) {
        var orig = 'url(' + uri + ')';   // <<<<<<< ERROR url(url(...))
[...]

So the bug is in component/builder2.js

jasonkuhrt commented 10 years ago

@jonathanong @callumlocke Hey guys at this time, a fix is required in one of or both:

Before diving into a pull-request I would like to vet the direction to take and if you want to fix yourselves.

A fix in component/builder2.js might look like:

      file.string = rewriteCSSURLs(string, function (justURL) {
        if (isData(justURL)) return justURL;
        if (isAbsolute(justURL)) return justURL;

        // IS THERE A BUG HERE TOO?
        justURL = resolve(file.path, justURL);
        justURL = resolve(prefix + utils.rewriteUrl(branch) + '/', justURL);
        return justURL;
      });
jonathanong commented 10 years ago

https://github.com/component/builder2.js/pull/49

jonathanong commented 10 years ago

k pushed to component-builder@1.1.3