Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignoring CSS from Urls with relative domains and https: #288

Closed
emiespo opened this issue Mar 14, 2018 · 3 comments
Closed

Ignoring CSS from Urls with relative domains and https: #288

emiespo opened this issue Mar 14, 2018 · 3 comments
Assignees

Comments

@emiespo
Copy link

emiespo commented Mar 14, 2018

Hi all,

I think I'll probably have to use penthouse directly, but I wanted to report this issue I came across.
Basically, when passing a url like this:

https://www.domain.com/

...and having CSS served from a different domain (edit: the URL is protocol-relative):

//www.cnd-domain.com/css/awesome-site.css

I get this warning in the output:

Ignoring http://www.cnd-domain.com/css/awesome-site.css (404)

(notice it tried http instead of https).

Looking at the code, I found this in critical/lib/file-helper.js:138:

        // Handle protocol-relative urls
        uri = url.resolve('http://te.st', uri);

Apparently it's assuming the url is not a secure one, it should rather get the same protocol as the main request (if any). Or maybe default to https? Changing that to https did the trick, btw.

As a side issue, I wasn't able to get the CDN path to appear in the inlined source (for images), is that possible at all? I've tried with the assetPaths option with no luck.

Thank you!

@pocketjoso
Copy link
Contributor

404 means there was no such file on the server. So this would normally mean it's a problem on your end (or the sites you're loading css from).

@emiespo
Copy link
Author

emiespo commented Mar 14, 2018

Read better: it is trying to access http instead of https (as it should be). If I change that line to https, it works :). My bad: I've edited the comment to explicit state the URL is protocol-relative (so it should NOT go to http, but https).

@pocketjoso
Copy link
Contributor

I see, then indeed it's a bug in critical.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants