Skip to content
This repository was archived by the owner on Apr 10, 2025. It is now read-only.

Commit e3aa7b7

Browse files
morlovichjeffkaufman
authored andcommitted
Relax CSS inlining charset check to always permit ASCII-only data.
Needed for fonts.g.c due to apparent recent changes (and us not permitting inlining there was making the corresponding continuous tests fail).
1 parent 2bd239d commit e3aa7b7

File tree

2 files changed

+39
-7
lines changed

2 files changed

+39
-7
lines changed

net/instaweb/rewriter/css_inline_filter.cc

Lines changed: 24 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -175,15 +175,34 @@ bool CssInlineFilter::ShouldInline(const ResourcePtr& resource,
175175
return false;
176176
}
177177

178-
// If the charset is incompatible with the HTML's, don't inline.
178+
// If the charset is incompatible with the HTML's, we may not be able to
179+
// inline.
179180
StringPiece htmls_charset(driver()->containing_charset());
180181
GoogleString css_charset = RewriteFilter::GetCharsetForStylesheet(
181182
resource.get(), attrs_charset, htmls_charset);
182183
if (!StringCaseEqual(htmls_charset, css_charset)) {
183-
*reason = StrCat("CSS not inlined due to apparent charset incompatibility;"
184-
" we think the HTML is ", htmls_charset,
185-
" while the CSS is ", css_charset);
186-
return false;
184+
// Check if everything is in <= 127 range, we may still be able to
185+
// inline if it keeps to the ASCII subset (also potentially dropping the
186+
// BOM, since we'll strip it anyway).
187+
StringPiece contents = resource->ExtractUncompressedContents();
188+
StringPiece clean_contents(contents);
189+
StripUtf8Bom(&clean_contents);
190+
191+
bool has_non_ascii = false;
192+
for (int i = 0, n = clean_contents.size(); i < n; ++i) {
193+
if (static_cast<unsigned char>(clean_contents[i]) >= 0x80) {
194+
has_non_ascii = true;
195+
break;
196+
}
197+
}
198+
199+
if (has_non_ascii) {
200+
*reason = StrCat(
201+
"CSS not inlined due to apparent charset incompatibility;"
202+
" we think the HTML is ", htmls_charset,
203+
" while the CSS is ", css_charset);
204+
return false;
205+
}
187206
}
188207

189208
return true;

net/instaweb/rewriter/css_inline_filter_test.cc

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -581,8 +581,8 @@ TEST_F(CssInlineFilterTest, InlineWithCompatibleBom) {
581581
"", css_with_bom, true, css, "");
582582
}
583583

584-
TEST_F(CssInlineFilterTest, DoNotInlineWithIncompatibleBom) {
585-
const GoogleString css = "BODY { color: red; }\n";
584+
TEST_F(CssInlineFilterTest, DoNotInlineWithIncompatibleBomAndNonAscii) {
585+
const GoogleString css = "BODY { color: red; /* \xD2\x90 */ }\n";
586586
const GoogleString css_with_bom = StrCat(kUtf8Bom, css);
587587
TestInlineCssWithOutputUrl("http://www.example.com/index.html",
588588
" <meta charset=\"ISO-8859-1\">\n",
@@ -594,6 +594,19 @@ TEST_F(CssInlineFilterTest, DoNotInlineWithIncompatibleBom) {
594594
"while the CSS is utf-8");
595595
}
596596

597+
TEST_F(CssInlineFilterTest, DoInlineWithIncompatibleBomAndAscii) {
598+
// Even though the content is labeled utf-8, it keeps to ASCII subset, so it's
599+
// safe to inline.
600+
const GoogleString css = "BODY { color: red; }\n";
601+
const GoogleString css_with_bom = StrCat(kUtf8Bom, css);
602+
TestInlineCssWithOutputUrl("http://www.example.com/index.html",
603+
" <meta charset=\"ISO-8859-1\">\n",
604+
"http://www.example.com/styles.css",
605+
"http://www.example.com/styles.css",
606+
"", css_with_bom, true, css,
607+
"");
608+
}
609+
597610
// See: http://www.alistapart.com/articles/alternate/
598611
// and http://www.w3.org/TR/html4/present/styles.html#h-14.3.1
599612
TEST_F(CssInlineFilterTest, AlternateStylesheet) {

0 commit comments

Comments
 (0)