@import support for arbitrarily nested files

remy / inliner

Node utility to inline images, CSS and JavaScript for a web page - useful for mobile sites

MIT License

1.1k stars 165 forks source link

@import support for arbitrarily nested files #12

Closed samstewart closed 9 years ago

samstewart commented 12 years ago

Remy, I'm an iOS developer who works for playhaven and we use your tool to inline some of our proprietary templates. Recently, my colleague noticed some problems with nested @imports. Namely, the the "url(..)"s were not being resolved correct (encoded and inlined).

After working with the code extensively, I found the problem in the getImportsCSS method. Basically, the original "flat substitution" method was problematic because the code must resolve the "url(...)" references relative to the @imported file just as the browser does. Hence, pinning the rooturl parameter at the topmost level was causing the problems.

Anyway, this is a rough first draft so please feel free to have me change anything. I tried to change as little as possible to stay inline (no pun intended) with the original codebase.

Best, Sam

remy commented 12 years ago

Do you have a sample url I can test this on? If not you can create a simple jsbin.com with the minimal example.

Otherwise this generally looks good. I've got a merge to with some offline code I have - which allows you to run the inliner on local files rather than limited to hosted urls - which might be of interest to you if you're using the tool.

samstewart commented 12 years ago

sure, let me whip together tomorrow. I'm currently slammed at work integrating the new tool with our build process.

samstewart commented 12 years ago

Remy, Apologies for the long delay, things have been crazy at work. My problem is simply the scale of our code base which we use inliner on.

Let me whip up a smaller, more specific test file tomorrow.

remy commented 12 years ago

Totally understand - and a super small simple test would be perfect.

I've pushed some changes that allow you to test local files so you can run inliner test.html rather than having to do a hosted solution.

I've also noticed some of my latest fixes doesn't let your pull request merge automatically - I'm sure there's nothing serious, but do take a quick look if you can.

Cheers so much.

On 9 Jan 2012, at 02:06, Sam Stewart reply@reply.github.com wrote:

Remy, Apologies for the long delay, things have been crazy at work. My problem is simply the scale of our code base which we use inliner on.

Let me whip up a smaller, more specific test file tomorrow.

Reply to this email directly or view it on GitHub: https://github.com/remy/inliner/pull/12#issuecomment-3406474

paulmars commented 11 years ago

I'm seeing this bug as well.

@import url("http://netdna.bootstrapcdn.com/twitter-bootstrap/2.3.0/css/bootstrap-combined.min.css");

ul {
  list-style: none;
}

This puts out a error log like this

get 404 on http://localhost:6070/(data:text/css

<head>
  <meta name="viewport" content="width=device-width, initial-scale=1, maximum-scale=1">
  <meta charset="UTF-8" />
  <title>App</title>
  <link rel="stylesheet" href="app.css" />
</head>

Removing the '@import' directive causes the entire thing to compress nicely.

paulmars commented 11 years ago

Tell me if there is anything you would like me to supply. Thanks.

remy commented 11 years ago

@paulmars if you can mock together a simple example that shows of the case I can bug fix against the simplified case.

paulmars commented 11 years ago

How's this look? https://gist.github.com/paulmars/f2fe4051ccbd846555d8

paulmars commented 11 years ago

I run it by typing "./node_modules/inliner/bin/inliner -nv http://localhost:6060/"

while running a locahost server. here is the output:

1.0.0.127.in-addr.arpa - - [29/Apr/2013 14:00:11] "GET / HTTP/1.1" 200 -
1.0.0.127.in-addr.arpa - - [29/Apr/2013 14:00:11] "GET /app.css HTTP/1.1" 200 -
1.0.0.127.in-addr.arpa - - [29/Apr/2013 14:00:11] code 404, message File not found
1.0.0.127.in-addr.arpa - - [29/Apr/2013 14:00:11] "GET /(data:text/css HTTP/1.1" 404 -

remy commented 11 years ago

Cheers - will check that out.

On 29 April 2013 22:02, Paul McKellar notifications@github.com wrote:

How's this look? https://gist.github.com/paulmars/f2fe4051ccbd846555d8

— Reply to this email directly or view it on GitHubhttps://github.com/remy/inliner/pull/12#issuecomment-17194130 .

paulmars commented 11 years ago

Appreciate any time you have. Thanks for inliner, it is saving me a bunch of time.

remy commented 9 years ago

This go long, long, long, long lost, but I've just released 1.0.0. I'm try adding your original gist to the test suite to make sure it's okay.

paulmars commented 9 years ago

wow. thanks