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

In amp pages inline_google_font_css must be disabled #1382

Closed
Lofesa opened this issue Feb 17, 2017 · 5 comments
Closed

In amp pages inline_google_font_css must be disabled #1382

Lofesa opened this issue Feb 17, 2017 · 5 comments

Comments

@Lofesa
Copy link

@Lofesa Lofesa commented Feb 17, 2017

When the filter inline_google_font_css is enabled, amp pages throw an error in amp test because the label <style> is not allowed but must be <style amp-custom> so in amp pages the filter must be disabled or label changed

@morlovich morlovich self-assigned this Feb 17, 2017
@morlovich
Copy link
Contributor

@morlovich morlovich commented Feb 21, 2017

Looks like only a single amp-custom is permitted, and we might not be able to tell if there is another one, so likely the best we can do is just autodisable...

@Lofesa
Copy link
Author

@Lofesa Lofesa commented Feb 21, 2017

Disabled better than make a rule to disable it in server config

@jansent
Copy link

@jansent jansent commented Feb 22, 2017

+1 on this. It'd be great if there was an easy way to combine the inlined font with the amp-custom inlined styles, or alternatively if amp spec was updated to allow for deferred styles or better support for their own fonts.

morlovich added a commit to apache/incubator-pagespeed-mod that referenced this issue Mar 8, 2017
morlovich added a commit to apache/incubator-pagespeed-mod that referenced this issue Mar 8, 2017
* Disable CSS (really font CSS in practice) inlining on AMP.

Ref: apache/incubator-pagespeed-ngx#1382

* Try to deflake the font inlining test some
@morlovich
Copy link
Contributor

@morlovich morlovich commented Mar 8, 2017

Should be fixed with that.

@morlovich morlovich closed this Mar 8, 2017
@jansent
Copy link

@jansent jansent commented Mar 8, 2017

Wow thanks!

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

Successfully merging a pull request may close this issue.

None yet
3 participants