search

andrewrk / juice

Juice inlines CSS stylesheets into your HTML source.
MIT License
60 stars 13 forks source link

Fixes for Windows paths #4

Open alexturpin opened 10 years ago

alexturpin commented 10 years ago

Nothing was working for me on Windows but was for others on Unix-like systems. I debugged it and found out that it couldn't open my file because of path resolving issues, and that was failing silently. So I made it throw an error instead of failing silently, and did some simplifying and replaced backslashes with forward slashes before resolving the full path.

andrewrk commented 10 years ago

Can you explain why the file:// logic didn't work? Looks like there was some Windows specific logic in there. What exactly was broken about it?

alexturpin commented 10 years ago

Yeah I should certainly have added more comments. As far as the error throw goes, my main point was to cause it to raise a fatal error, but maybe we could figure out a better way to pass it up and show it to the user.

I found that the whole detecting of file:// to determine whether the file was local or not was completely useless as it always prepended file:// before. In my opinion, it should look for protocol in the link to the stylesheet and determine that, which is what I did.

The actual problem with Windows is that url.resolve doesn't work with backslashes. Take this for example:

url.resolve("c:\src\test\index.html", "assets/style.css")

Which is what was happening with me. That gives a weird results unless the backslashes of the first path are converted to forward slashes.

Anyhow, those commits fixed everything for me and I'll be using my own fork for now, but figured I'd try and contribute a bit. I can help flesh it out better if needed.

andrewrk commented 10 years ago

I appreciate the contribution. I'll dig into this later when I have some time.

adam-lynch commented 10 years ago

Possibly related? https://github.com/Automattic/juice/pull/35#issuecomment-47364321