Disable CSS (really font CSS in practice) inlining on AMP. (#1499)
* Disable CSS (really font CSS in practice) inlining on AMP.
Ref: https://github.com/pagespeed/ngx_pagespeed/issues/1382
* Try to deflake the font inlining test some
diff --git a/net/instaweb/rewriter/css_inline_filter.cc b/net/instaweb/rewriter/css_inline_filter.cc
index 9e37e18..243a474 100644
--- a/net/instaweb/rewriter/css_inline_filter.cc
+++ b/net/instaweb/rewriter/css_inline_filter.cc
@@ -117,6 +117,16 @@
const char* media = NULL;
if (CssTagScanner::ParseCssElement(element, &href, &media) &&
!driver()->HasChildrenInFlushWindow(element)) {
+ if (driver()->is_amp_document()) {
+ // Don't inline into AMP documents. Those do permit font loading CSS,
+ // which we could in principle inline, but they also restrict the document
+ // to a single <style> tag, and we don't really have a good way of
+ // coordinating everything into that.
+ driver()->InsertDebugComment(
+ "CSS inlining not supported by PageSpeed for AMP documents", element);
+ return;
+ }
+
// Only inline if the media type affects "screen". We don't inline other
// types since they're very unlikely to change the initial page view, and
// inlining them would actually slow down the 99% case of "screen".
diff --git a/net/instaweb/rewriter/css_inline_filter_test.cc b/net/instaweb/rewriter/css_inline_filter_test.cc
index ea92cab..cf18524 100644
--- a/net/instaweb/rewriter/css_inline_filter_test.cc
+++ b/net/instaweb/rewriter/css_inline_filter_test.cc
@@ -156,6 +156,8 @@
server_context()->ComputeSignature(options());
}
+ bool AddHtmlTags() const override { return false; }
+
private:
bool filters_added_;
};
@@ -710,6 +712,25 @@
VerifyNoInliningForClosingStyleTag("</style abc>");
}
+TEST_F(CssInlineFilterTest, DisabledForAmp) {
+ AddFilter(RewriteOptions::kInlineCss);
+ SetResponseWithDefaultHeaders("foo.css", kContentTypeCss,
+ "/* pretend there is a @font-face here */",
+ 100);
+ TurnOnDebug();
+ ValidateExpected(
+ "no_inlining_in_amp",
+ "<html amp><link rel='stylesheet' href='foo.css'>",
+ "<html amp><link rel='stylesheet' href='foo.css'>"
+ "<!--CSS inlining not supported by PageSpeed for AMP documents-->");
+
+ // Make sure same stylesheet gets inlined elsewhere.
+ ValidateExpected(
+ "same_url_in_non_amp",
+ "<link rel='stylesheet' href='foo.css'>",
+ "<style>/* pretend there is a @font-face here */</style>");
+}
+
} // namespace
} // namespace net_instaweb
diff --git a/pagespeed/automatic/system_tests/inliners.sh b/pagespeed/automatic/system_tests/inliners.sh
index db55fa3..a4f5cb7 100644
--- a/pagespeed/automatic/system_tests/inliners.sh
+++ b/pagespeed/automatic/system_tests/inliners.sh
@@ -28,9 +28,8 @@
user_agent =Mozilla/5.0 (X11; Linux x86_64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.36 Safari/537.36
EOF
- fetch_until $URL 'grep -c @font-face' 7
-
- OUT=$($WGET_DUMP $URL)
+ fetch_until -save $URL 'grep -c @font-face' 7 "" -ge
+ OUT=$(cat $FETCH_UNTIL_OUTFILE)
check_from "$OUT" fgrep -qi "format('woff2')"
check_not_from "$OUT" fgrep -qi "format('truetype')"
check_not_from "$OUT" fgrep -qi "format('embedded-opentype')"
@@ -44,10 +43,10 @@
user_agent = Mozilla/4.0 (compatible; MSIE 6.01; Windows NT 6.0)
EOF
- fetch_until $URL 'grep -c @font-face' 1
+ fetch_until -save $URL 'grep -c @font-face' 1
# This should get an eot font. (It might also ship a woff, so we don't
# check_not_from for that)
- OUT=$($WGET_DUMP $URL)
+ OUT=$(cat $FETCH_UNTIL_OUTFILE)
check_from "$OUT" fgrep -qi ".eot"
check_not_from "$OUT" fgrep -qi ".ttf"
