Skip to content

Fetch CSS over HTTP when URL lacks extension; convert font CDN stylesheets @imports to convert to links instead of fetching#1357

Merged
westonruter merged 3 commits intodevelopfrom
fix/external-stylesheet-imports
Aug 27, 2018
Merged

Fetch CSS over HTTP when URL lacks extension; convert font CDN stylesheets @imports to convert to links instead of fetching#1357
westonruter merged 3 commits intodevelopfrom
fix/external-stylesheet-imports

Conversation

@westonruter
Copy link
Copy Markdown
Member

@westonruter westonruter commented Aug 25, 2018

If there is a stylesheet link with a URL which does not end in css, fetch it over HTTP instead of throwing a validation error.

Additionally, if there is a stylesheet that contains an @import to a stylesheet URL which is whitelisted CDN for fonts such as:

@import "https://fonts.googleapis.com/css?family=Merriweather:300";

Then instead of fetching the CSS to inline, opt instead to convert into a normal link tag:

<link rel="stylesheet" href="https://fonts.googleapis.com/css?family=Merriweather:300">

Fixes #1317

@westonruter westonruter added this to the v1.0 milestone Aug 25, 2018
@westonruter westonruter changed the title [WIP] Convert font CDN stylesheets @imports to convert to links; allow fetch external CSS without extensions Fetch CSS over HTTP when URL lacks extension; convert font CDN stylesheets @imports to convert to links instead of fetching Aug 25, 2018
Copy link
Copy Markdown
Contributor

@hellofromtonya hellofromtonya left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. Great job @westonruter

@westonruter westonruter merged commit a6f8eae into develop Aug 27, 2018
@westonruter westonruter deleted the fix/external-stylesheet-imports branch August 27, 2018 21:45
@westonruter
Copy link
Copy Markdown
Member Author

See also Automattic/nginx-http-concat#41 which would bypasse VIP's http-concat for CSS when is_amp_endpoint(). Having this in place will improve the AMP integration yet further, as it would eliminate redundant and unnecessary work.

@kienstra
Copy link
Copy Markdown
Contributor

Moving To 'Ready For Merging'

If it's alright, I'm moving this to 'Ready For Merging,' as I don't think it needs functional testing. Feel free to move it back.

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

Successfully merging this pull request may close these issues.

3 participants