search

rgrove / combohandler

A simple Yahoo!-style combo handler in Node.js.
MIT License
116 stars 32 forks source link

CSS url rewriting is broken (on windows) #35

Open MarkKharitonov opened 10 years ago

MarkKharitonov commented 10 years ago

The CSS url rewrite does not seem to work. Here is my web directory:

C:\work\if\server\web>dir
 Volume in drive C has no label.
 Volume Serial Number is 82A9-DF6B

 Directory of C:\work\if\server\web

01/10/2014  06:06 AM    <DIR>          .
01/10/2014  06:06 AM    <DIR>          ..
01/02/2014  07:15 AM    <DIR>          css
01/10/2014  06:06 AM    <JUNCTION>     if [C:\work\jetty\webapps\if]
01/02/2014  07:49 AM    <DIR>          js
01/04/2014  11:58 AM    <DIR>          lib
01/10/2014  06:05 AM    <JUNCTION>     yui [C:\work\jetty\webapps\yui]
01/10/2014  06:06 AM    <JUNCTION>     yui-gallery [C:\work\jetty\webapps\yui-gallery]
               0 File(s)              0 bytes
               8 Dir(s)  35,535,917,056 bytes free

C:\work\if\server\web>

_dirname is C:\work\if\server The project loads yui/datatable-sort/assets/skins/sam/datatable-sort.css, which contains the following line:

url(../../../../assets/skins/sam/sprite.png)

Try 1:

app.get('/combo',
  combo.combine({
    rootPath: __dirname + '/web/',
    basePath: '/web'
  }),
  combo.respond);

The combined css rewrites the aforementioned url directive to

url(C:\web\yui\assets\skins\sam\sprite.png)

Try 2:

app.get('/combo',
  combo.combine({
    rootPath: __dirname + '/web/',
    webRoot: __dirname
  }),
  combo.respond);

Which yields exactly the same url directive:

url(C:\web\yui\assets\skins\sam\sprite.png)

Of course, it does not work.

MarkKharitonov commented 10 years ago

Forgot to mention the YUI_config:

YUI_config = {
  combine: true,
  filter: 'debug',
  comboBase: 'http://localhost:8000/combo?',
  root: 'yui/',
  base: 'http://localhost:8000/yui/',
  maxURLLength: 4000,
  groups: {
    gallery: {
      combine: true,
      filter: 'debug',
      root: 'yui-gallery/',
      base: 'http://localhost:8000/yui-gallery/',
      patterns: {
        "gallery-": {},
        "gallerycss-": { type: "css" }
      }
    }
  }
};

I shortened the nesting of the yui and yui-gallery folders by moving the content from under the build folder to be direct children of yui or yui-gallery:

C:\work\if\server\web>dir yui
 Volume in drive C has no label.
 Volume Serial Number is 82A9-DF6B

 Directory of C:\work\if\server\web\yui

01/01/2014  08:55 AM    <DIR>          .
01/01/2014  08:55 AM    <DIR>          ..
09/29/2013  11:46 AM    <DIR>          align-plugin
09/29/2013  11:46 AM    <DIR>          anim-base
09/29/2013  11:46 AM    <DIR>          anim-color
09/29/2013  11:46 AM    <DIR>          anim-curve
...

yui-gallery was treated in a similar fashion.

MarkKharitonov commented 10 years ago

I have created a pull request (https://github.com/rgrove/combohandler/pull/36) addressing this issue. In addition, the pull request adds baseUrl config option to allow for scenarios where the static content and the dynamic content is served from different servers.

evocateur commented 10 years ago

Thanks so much for looking into this, I apologize for not responding earlier on the yui-support thread. Windows is definitely our Achilles' heel on this project, as neither myself nor @rgrove develop on or run the service in a Windows environment. I'll get the pull request sorted straightaway.

evocateur commented 10 years ago

@MarkKharitonov What happens when you provide a fully-valid Windows rootPath (via path.join) when using the webRoot config?

var path = require('path');

// ...
app.get('/combo',
  combo.combine({
    rootPath: path.join(__dirname, 'web'),
    webRoot: __dirname
  }),
  combo.respond);

(as an aside, webRoot is generally what you want, basePath is an old, confusing config that is all but deprecated now)

MarkKharitonov commented 10 years ago

Please, see Try 2 described in the body of the issue above. It contains the answer to your question.

evocateur commented 10 years ago

Actually, no it doesn't. I asked you if the valid path created by path.join(__dirname, "web") worked in the webRoot config, not a concatenation of __dirname and "/web/" (because the plain concatenation would result in a path that had both windows and unix path seperators in it).

In any case, I can't accept this patch until the tests pass. It will be Monday before I can attempt to spin up a Windows VM and fix the tests myself.

On Jan 11, 2014, at 19:04, MarkKharitonov notifications@github.com wrote:

Please, see Try 2 described in the body of the issue above. It contains the answer to your question.

— Reply to this email directly or view it on GitHub.

MarkKharitonov commented 10 years ago

Using either path.join or concat ultimately yield the same result, because the code calls path.normalize at the end.

Also note that Windows recognizes both kinds of slashes as path separators.

Le samedi 11 janvier 2014, Daniel Stockman a écrit :

Actually, no it doesn't. I asked you if the valid path created by path.join(__dirname, "web") worked in the webRoot config, not a concatenation of __dirname and "/web/" (because the plain concatenation would result in a path that had both windows and unix path seperators in it).

In any case, I can't accept this patch until the tests pass. It will be Monday before I can attempt to spin up a Windows VM and fix the tests myself.

On Jan 11, 2014, at 19:04, MarkKharitonov <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>> wrote:

Please, see Try 2 described in the body of the issue above. It contains the answer to your question.

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHubhttps://github.com/rgrove/combohandler/issues/35#issuecomment-32114356 .

Be well and prosper.

"There are two kinds of people.Those whose guns are loaded and those who dig." ("The good, the bad and the ugly") So let us drink for our guns always be loaded.

evocateur commented 10 years ago

Thanks, I can rule that out now. I didn't know Windows accepted both delimiters. Crazy.

On Sat, Jan 11, 2014 at 7:53 PM, MarkKharitonov notifications@github.com wrote:

Using either path.join or concat ultimately yield the same result, because the code calls path.normalize at the end. Also note that Windows recognizes both kinds of slashes as path separators. Le samedi 11 janvier 2014, Daniel Stockman a écrit :

Actually, no it doesn't. I asked you if the valid path created by path.join(__dirname, "web") worked in the webRoot config, not a concatenation of __dirname and "/web/" (because the plain concatenation would result in a path that had both windows and unix path seperators in it).

In any case, I can't accept this patch until the tests pass. It will be Monday before I can attempt to spin up a Windows VM and fix the tests myself.

On Jan 11, 2014, at 19:04, MarkKharitonov <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>> wrote:

Please, see Try 2 described in the body of the issue above. It contains the answer to your question.

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHubhttps://github.com/rgrove/combohandler/issues/35#issuecomment-32114356 .

Be well and prosper.

"There are two kinds of people.Those whose guns are loaded and those who dig." ("The good, the bad and the ugly")

So let us drink for our guns always be loaded.

Reply to this email directly or view it on GitHub: https://github.com/rgrove/combohandler/issues/35#issuecomment-32114588

MarkKharitonov commented 10 years ago

http://superuser.com/questions/176388/why-does-windows-use-backslashes-for-paths-and-unix-forward-slashes