Permalink
Browse files

image-inline: don't inline shortcut images

Because shortcut images don't block page rendering and generally aren't that important the tradeoffs are different for them, and we shouldn't inline them.

Fixes #1022
  • Loading branch information...
jeffkaufman authored and crowell committed Jun 23, 2016
1 parent dbde989 commit 203865af81afe5ee6c18f555a4d4ebfb2f0da607
@@ -0,0 +1,9 @@
<link rel="icon"
href="../mod_pagespeed_example/images/Cuppa.png">
<link rel="apple-touch-icon"
href="../mod_pagespeed_example/images/Cuppa.png">
<link rel="apple-touch-icon-precomposed"
href="../mod_pagespeed_example/images/Cuppa.png">
<link rel="apple-touch-startup-image"
href="../mod_pagespeed_example/images/Cuppa.png">
<img src="../mod_pagespeed_example/images/Cuppa.png">
@@ -365,6 +365,9 @@ const char* MessageForInlineResult(InlineResult inline_result) {
// image will be deleted before the user sees it, so message won't be
// useful.
break;
case INLINE_SHORTCUT:
message = "The image was not inlined because it is a shortcut icon.";
break;
case INLINE_INTERNAL_ERROR:
message = "The image was not inlined because the internal data was "
"corrupted.";
@@ -1596,6 +1599,15 @@ bool ImageRewriteFilter::FinishRewriteImageUrl(
StringPiece(responsive_attr) !=
ResponsiveImageFirstFilter::kInlinableVirtualImage) {
*inline_result = INLINE_RESPONSIVE;
} else if (element->keyword() == HtmlName::kLink) {
// Don't inline shortcut images. All shortcut images are on link tags, and
// no non-shortcut images are on link tags, so we can just check if this is
// a link tag. This is to exclude inlining on:
// * <link rel=icon ...>
// * <link rel=apple-touch-icon ...>
// * <link rel=apple-touch-icon-precomposed ...>
// * <link rel=apple-touch-startup-image ...>
*inline_result = INLINE_SHORTCUT;
} else {
// See if we have a data URL, and if so use it if the browser can handle it
// TODO(jmaessen): get rid of a string copy here. Tricky because
@@ -44,6 +44,7 @@

namespace net_instaweb {

// See MessageForInlineResult for enum meanings.
enum InlineResult {
INLINE_SUCCESS,
INLINE_UNSUPPORTED_DEVICE,
@@ -53,6 +54,7 @@ enum InlineResult {
INLINE_CACHE_SMALL_IMAGES_UNREWRITTEN,
// Image should not be inlined because it is part of a responsive image.
INLINE_RESPONSIVE,
INLINE_SHORTCUT,
INLINE_INTERNAL_ERROR,
};

@@ -0,0 +1,29 @@
start_test "shortcut icons aren't inlined"

URL="$TEST_ROOT/shortcut_icons.html?PageSpeedFilters=debug,rewrite_images"

# We expect to see just one inline image, because of the img tag.
fetch_until -save $URL 'grep -c data:image/png' 1

OUT=$(cat $FETCH_UNTIL_OUTFILE)

# All four icon link tags should be optimized but not inlined.
check_from "$OUT" grep \
'<link rel="icon" href="[^<]*xCuppa.png.pagespeed.ic'
check_from "$OUT" grep \
'<link rel="apple-touch-icon" href="[^<]*xCuppa.png.pagespeed.ic'
check_from "$OUT" grep \
'<link rel="apple-touch-icon-precomposed" href="[^<]*xCuppa.png.pagespeed.ic'
check_from "$OUT" grep \
'<link rel="apple-touch-startup-image" href="[^<]*xCuppa.png.pagespeed.ic'

# The image tag should have been inlined instead.
check_not_from "$OUT" grep \
'<img src="[^<]*xCuppa.png.pagespeed.ic'

function appears_exactly_four_times() {
test $(grep -c "$@") = 4
}

check_from "$OUT" appears_exactly_four_times \
"The image was not inlined because it is a shortcut icon."
@@ -57,6 +57,7 @@ run_test make_show_ads_async
# Disable mobilizer tests.
# run_test mobilizer
run_test responsive_images
run_test shortcut_icons

# These have to run after image_rewrite tests. Otherwise it causes some images
# to be loaded into memory before they should be.
@@ -0,0 +1,29 @@
start_test "shortcut icons aren't inlined"

URL="$TEST_ROOT/shortcut_icons.html?PageSpeedFilters=debug,rewrite_images"

# We expect to see just one inline image, because of the img tag.
fetch_until -save $URL 'grep -c data:image/png' 1

OUT=$(cat $FETCH_UNTIL_OUTFILE)

# All four icon link tags should be optimized but not inlined.
check_from "$OUT" grep \
'<link rel="icon" href="[^<]*xCuppa.png.pagespeed.ic'
check_from "$OUT" grep \
'<link rel="apple-touch-icon" href="[^<]*xCuppa.png.pagespeed.ic'
check_from "$OUT" grep \
'<link rel="apple-touch-icon-precomposed" href="[^<]*xCuppa.png.pagespeed.ic'
check_from "$OUT" grep \
'<link rel="apple-touch-startup-image" href="[^<]*xCuppa.png.pagespeed.ic'

# The image tag should have been inlined instead.
check_not_from "$OUT" grep \
'<img src="[^<]*xCuppa.png.pagespeed.ic'

function appears_exactly_four_times() {
test $(grep -c "$@") = 4
}

check_from "$OUT" appears_exactly_four_times \
"The image was not inlined because it is a shortcut icon."

0 comments on commit 203865a

Please sign in to comment.