itgalaxy / favicons

Favicons generator for Node.js
MIT License
1.19k stars 164 forks source link

Absolute url #103

Closed minhtranite closed 8 years ago

minhtranite commented 8 years ago
<meta property="twitter:image" content="http:/localhost:3000/9aa5cd46d7f431d2838bef208ec125ab.png">
<meta property="og:image" content="http:/localhost:3000/5daf4bea0e3ab132107fa0c27a5dfe52.png">

I think url shoud be http://localhost:3000 instead of http:/localhost:3000

haydenbleasel commented 8 years ago

I thought this was fixed in #73 / #74. What versions of Favicons / Gulp-Favicons are you using?

minhtranite commented 8 years ago

You can see more info here.

minhtranite commented 8 years ago

I think bug come from line: https://github.com/haydenbleasel/favicons/blob/master/helpers.js#L42. When call replace it replace // by /.

minhtranite commented 8 years ago

It better than replace two time.

...
function relative (directory) {
            return path.join(options.path, directory).replace(/\\/g, '/');
        }

        function absolute (directory) {
            return path.join(options.url, relative(directory));
        }
...
andrey-hohlov commented 8 years ago

My mistake :(

haydenbleasel commented 8 years ago

I think this is also due to path.join which is meant to be url.resolve (to preserve the double slashes). This keeps reverting in new commits so I'll need to make sure this is fixed.

haydenbleasel commented 8 years ago

Fixed in favicons@4.5.2

jeffreywescott commented 7 years ago

I'm using version 4.8.6 and still seeing this. I'm not using Gulp -- just using the code in my own app directly.

jeffreywescott commented 7 years ago

To reproduce, just pass a URL in the path option, like this: 'http://example.com'.

jeffreywescott commented 7 years ago

Workaround is to use a base URL like this: 'http:\\\\example.com' -- stupid ugly and confusing, but works.