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

Fetch external stylesheets (which aren't from whitelisted font CDNs) to include in amp-custom style #1083

Closed
westonruter opened this issue Apr 19, 2018 · 0 comments
Assignees
Milestone

Comments

@westonruter
Copy link
Member

Currently when coming across a <link rel=stylesheet> we check if it has an href pointing to an allowed font URL and then it gets allowed, but if it has an href that points somewhere else on an external domain then it gets removed:

https://github.com/Automattic/amp-wp/blob/ac53ee79cba5d93ce1a89d37f303f4121e4566fb/includes/sanitizers/class-amp-style-sanitizer.php#L431-L447

https://github.com/Automattic/amp-wp/blob/ac53ee79cba5d93ce1a89d37f303f4121e4566fb/includes/sanitizers/class-amp-style-sanitizer.php#L348-L352

This can result in styles being broken on a site if they depend on the external styles.

I suggest that instead of just removing the style that we instead fetch the contents of the stylesheet and store it in a transient (with cache key being hash of stylesheet URL) which expires according to the Expires time returned in the response or MONTH_IN_SECONDS, whichever is greater. What we must absolutely avoid is the external stylesheet being fetched and refetched with each request.

See #1082 where a workaround prototype plugin has been put in place until 1.0 lands: https://gist.github.com/westonruter/f272303fc4bf2d5d71fe1bd88a5fcee3

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

No branches or pull requests

3 participants