Permalink
Browse files

improved handling of inlined and multiple bg images and sprites

ref: futtta#87

* adds two new filters for overriding/shortcircuiting image datauri inlining logic
* sort replacements array by key length in desc order (in fixurls() and rewrite_assets())
* adds crude tests for inlining and sprites etc. by using the new filters
  • Loading branch information...
1 parent 58d35be commit a39edcbb5c465b8797992776d5fbf296b08ae545 @zytzagoo committed Mar 6, 2017
Showing with 179 additions and 3 deletions.
  1. +25 −3 classes/autoptimizeStyles.php
  2. +154 −0 tests/test-ao.php
@@ -198,11 +198,13 @@ private function is_datauri_candidate($path)
file_exists( $path ) && is_readable( $path ) && filesize( $path ) <= $max_size ) {
// Seems we have a candidate
- return true;
+ $is_candidate = true;
+ } else {
+ // Filter allows overriding default decision (which checks for local file existence)
+ $is_candidate = apply_filters( 'autoptimize_filter_css_is_datauri_candidate', false, $path );
}
- // Noup, anything else ain't a candidate
- return false;
+ return $is_candidate;
}
/**
@@ -258,6 +260,18 @@ private function check_datauri_exclude_list($url)
private function build_or_get_datauri_image($path)
{
+ // TODO/FIXME: document the required return array format, or better yet,
+ // use a string, since we don't really need an array for this. That would, however,
+ // require changing even more code, which is not happening right now...
+
+ // Allows short-circuiting datauri generation for an image
+ $result = apply_filters( 'autoptimize_filter_css_datauri_image', array(), $path );
+ if ( ! empty( $result ) ) {
+ if ( is_array( $result) && isset( $result['full'] ) && isset( $result['base64data'] ) ) {
+ return $result;
+ }
+ }
+
$hash = md5( $path );
$check = new autoptimizeCache($hash, 'img');
if ( $check->check() ) {
@@ -360,6 +374,10 @@ public function rewrite_assets($code)
}
if ( ! empty( $imgreplace ) ) {
+ // Sort the replacements array by key length in desc order (so that the longest strings are replaced first)
+ $keys = array_map( 'strlen', array_keys( $imgreplace) );
+ array_multisort( $keys, SORT_DESC, $imgreplace );
+
$this->debug_log($imgreplace);
$code = str_replace( array_keys( $imgreplace ), array_values( $imgreplace ), $code );
}
@@ -750,6 +768,10 @@ static function fixurls($file, $code)
}
if ( ! empty( $replace ) ) {
+ // Sort the replacements array by key length in desc order (so that the longest strings are replaced first)
+ $keys = array_map( 'strlen', array_keys( $replace ) );
+ array_multisort( $keys, SORT_DESC, $replace );
+
// Replace URLs found within $code
$code = str_replace( array_keys( $replace ), array_values( $replace ), $code );
}
View
@@ -855,4 +855,158 @@ public function test_fixurls_with_hash_only_urls()
$fixurls_result = autoptimizeStyles::fixurls(ABSPATH . 'wp-content/themes/my-theme/style.css', $css_orig);
$this->assertEquals($css_expected, $fixurls_result);
}
+
+ public function test_background_datauri_sprites_with_fixurls()
+ {
+ $css_orig = <<<CSS
+.shadow { background:url(img/1x1.png) top center; }
+.shadow1 { background-image:url(img/1x1.png) 0 -767px repeat-x; }
+.shadow2 {background:url(img/1x1.png) top center}
+
+.test { background:url(img/1x1.png) top center; }
+.test1 { background-image:url('img/1x1.png') 0 -767px repeat-x; }
+.test2 {background:url("img/1x1.png") top center}
+
+header{background-image:url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='90px' height='110px' viewBox='0 0 90 110'%3E%3Cstyle%3E.a%7Bstop-color:%23FFF;%7D.b%7Bstop-color:%23B2D235;%7D.c%7Bstop-color:%23BEE7FA;%7D.d%7Bfill:%23590C15;%7D%3C/style%3E%3ClinearGradient id='c' y2='135.4' gradientUnits='userSpaceOnUse' x2='209.1' gradientTransform='rotate(-1.467 -4082.888 7786.794)' y1='205.8' x1='262'%3E%3Cstop class='b' offset='0'/%3E%3Cstop class='b' offset='.48'/%3E%3Cstop stop-color='%23829D25' offset='1'/%3E%3C/linearGradient%3E%3Cpath stroke-width='.3' d='M77.3 45.4c-3-3.5-7.1-6.5-11.6-7.8-5.1-1.5-10-.1-14.9 1.5C52 35.4 54.3 29 60 24l-4.8-5.5c-3.4 3-5.8 6.3-7.5 9.4-1.7-4.3-4.1-8.4-7.5-12C33.4 8.6 24.3 4.7 15.1 4.2c-.2 9.3 3.1 18.6 9.9 25.9 5.2 5.6 11.8 9.2 18.7 10.8-2.5.2-4.9-.1-7.7-.9-5.2-1.4-10.5-2.8-15.8-1C10.6 42.3 4.5 51.9 4 61.7c-.5 11.6 3.8 23.8 9.9 33.5 3.9 6.3 9.6 13.7 17.7 13.4 3.8-.1 7-2.1 10.7-2.7 5.2-.8 9.1 1.2 14.1 1.8 16.4 2 24.4-23.6 26.4-35.9 1.2-9.1.8-19.1-5.5-26.4z' stroke='%233E6D1F' fill='url(%23c)'/%3E%3C/svg%3E")}
+
+/*
+section.clipped.clippedTop {clip-path:url("#clipPolygonTop")}
+section.clipped.clippedBottom {clip-path:url("#clipPolygonBottom")}
+.myimg {background-image: url("images/under-left-leaf.png"), url("images/over-blue-bird.png"), url("images/under-top.png"), url("images/bg-top-grunge.png");}
+.something {
+ background:url(http://example.org/wp-content/themes/my-theme/images/nothing.png);
+}
+.something-else {background:url(wp-content/themes/my-theme/images/shadow.png) -100px 0 repeat-y;}
+.another-thing { background:url(/wp-content/themes/my-theme/images/shadow.png) 0 -767px repeat-x; }
+#whatevz {background:url(wp-content/themes/my-theme/images/shadow.png) center top no-repeat;}
+
+.widget ul li { background:url(img/shadow.png) top center; }
+*/
+CSS;
+ $css_expected = <<<CSS
+.shadow { background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) top center; }
+.shadow1 { background-image:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) 0 -767px repeat-x; }
+.shadow2 {background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) top center}
+
+.test { background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) top center; }
+.test1 { background-image:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) 0 -767px repeat-x; }
+.test2 {background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) top center}
+
+header{background-image:url("data:image/svg+xml,%3Csvg xmlns='http://www.w3.org/2000/svg' width='90px' height='110px' viewBox='0 0 90 110'%3E%3Cstyle%3E.a%7Bstop-color:%23FFF;%7D.b%7Bstop-color:%23B2D235;%7D.c%7Bstop-color:%23BEE7FA;%7D.d%7Bfill:%23590C15;%7D%3C/style%3E%3ClinearGradient id='c' y2='135.4' gradientUnits='userSpaceOnUse' x2='209.1' gradientTransform='rotate(-1.467 -4082.888 7786.794)' y1='205.8' x1='262'%3E%3Cstop class='b' offset='0'/%3E%3Cstop class='b' offset='.48'/%3E%3Cstop stop-color='%23829D25' offset='1'/%3E%3C/linearGradient%3E%3Cpath stroke-width='.3' d='M77.3 45.4c-3-3.5-7.1-6.5-11.6-7.8-5.1-1.5-10-.1-14.9 1.5C52 35.4 54.3 29 60 24l-4.8-5.5c-3.4 3-5.8 6.3-7.5 9.4-1.7-4.3-4.1-8.4-7.5-12C33.4 8.6 24.3 4.7 15.1 4.2c-.2 9.3 3.1 18.6 9.9 25.9 5.2 5.6 11.8 9.2 18.7 10.8-2.5.2-4.9-.1-7.7-.9-5.2-1.4-10.5-2.8-15.8-1C10.6 42.3 4.5 51.9 4 61.7c-.5 11.6 3.8 23.8 9.9 33.5 3.9 6.3 9.6 13.7 17.7 13.4 3.8-.1 7-2.1 10.7-2.7 5.2-.8 9.1 1.2 14.1 1.8 16.4 2 24.4-23.6 26.4-35.9 1.2-9.1.8-19.1-5.5-26.4z' stroke='%233E6D1F' fill='url(%23c)'/%3E%3C/svg%3E")}
+
+/*
+section.clipped.clippedTop {clip-path:url("#clipPolygonTop")}
+section.clipped.clippedBottom {clip-path:url("#clipPolygonBottom")}
+.myimg {background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=), url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=), url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=), url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=);}
+.something {
+ background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=);
+}
+.something-else {background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) -100px 0 repeat-y;}
+.another-thing { background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) 0 -767px repeat-x; }
+#whatevz {background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) center top no-repeat;}
+
+.widget ul li { background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) top center; }
+*/
+CSS;
+
+ // For test purposes, ALL images in the css are being inline with a 1x1 trans png string/datauri
+ add_filter( 'autoptimize_filter_css_is_datauri_candidate', function($is_candidate, $path) {
+ return true;
+ }, 10, 2);
+
+ // For test purposes, ALL images in the css are being inline with a 1x1 trans png string/datauri
+ add_filter( 'autoptimize_filter_css_datauri_image', function($base64array, $path) {
+ $head = 'data:image/png;base64,';
+ $data = 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=';
+
+ $result['full'] = $head . $data;
+ $result['base64data'] = $data;
+ return $result;
+ }, 10, 2);
+
+ $instance = new autoptimizeStyles($css_orig);
+ $instance->setOption('datauris', true);
+
+ $fixurls_result = autoptimizeStyles::fixurls(ABSPATH . 'wp-content/themes/my-theme/style.css', $css_orig);
+ $css_actual = $instance->rewrite_assets($fixurls_result);
+
+ $this->assertEquals($css_expected, $css_actual);
+ }
+
+ /**
+ * Doing rewrite_assets() without calling fixurls() beforehand could cause wrong results if/when there's a
+ * (same) image referenced via root-relative and relative urls, i.e., `/wp-content/themes/my-theme/images/shadow.png` and
+ * `wp-content/themes/my-theme/images/shadow.png` in test code below.
+ * That's because urls are not really "normalized" in rewrite_assets() at all, and replacements are done
+ * using simple string keys (based on url), so whenever the shorter url ends up being spotted first, the replacement
+ * was done in a way that leaves the first `/` character in place. Which could mean trouble, especially when
+ * doing inlining of smaller images.
+ * After sorting the replacements array in rewrite_assets() by string length in descending order, the problem
+ * goes away.
+ */
+ public function test_background_datauri_sprites_without_fixurls()
+ {
+ $css_orig = <<<CSS
+.shadow { background:url(img/1x1.png) top center; }
+.shadow1 { background-image:url(img/1x1.png) 0 -767px repeat-x; }
+.shadow2 {background:url(img/1x1.png) top center}
+
+.test { background:url(img/1x1.png) top center; }
+.test1 { background-image:url('img/1x1.png') 0 -767px repeat-x; }
+.test2 {background:url("img/1x1.png") top center}
+
+section.clipped.clippedTop {clip-path:url("#clipPolygonTop")}
+section.clipped.clippedBottom {clip-path:url("#clipPolygonBottom")}
+.myimg {background-image: url("images/under-left-leaf.png"), url("images/over-blue-bird.png"), url("images/under-top.png"), url("images/bg-top-grunge.png");}
+.something {
+ background:url(http://example.org/wp-content/themes/my-theme/images/nothing.png);
+}
+.something-else {background:url(wp-content/themes/my-theme/images/shadow.png) -100px 0 repeat-y;}
+.another-thing { background:url(/wp-content/themes/my-theme/images/shadow.png) 0 -767px repeat-x; }
+#whatevz {background:url(wp-content/themes/my-theme/images/shadow.png) center top no-repeat;}
+
+.widget ul li { background:url(img/shadow.png) top center; }
+CSS;
+ $css_expected = <<<CSS
+.shadow { background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) top center; }
+.shadow1 { background-image:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) 0 -767px repeat-x; }
+.shadow2 {background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) top center}
+
+.test { background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) top center; }
+.test1 { background-image:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) 0 -767px repeat-x; }
+.test2 {background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) top center}
+
+section.clipped.clippedTop {clip-path:url("#clipPolygonTop")}
+section.clipped.clippedBottom {clip-path:url("#clipPolygonBottom")}
+.myimg {background-image: url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=), url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=), url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=), url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=);}
+.something {
+ background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=);
+}
+.something-else {background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) -100px 0 repeat-y;}
+.another-thing { background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) 0 -767px repeat-x; }
+#whatevz {background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) center top no-repeat;}
+
+.widget ul li { background:url(data:image/png;base64,iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=) top center; }
+CSS;
+
+ // For test purposes, ALL images in the css are being inline with a 1x1 trans png string/datauri
+ add_filter( 'autoptimize_filter_css_is_datauri_candidate', function($is_candidate, $path) {
+ return true;
+ }, 10, 2);
+
+ // For test purposes, ALL images in the css are being inline with a 1x1 trans png string/datauri
+ add_filter( 'autoptimize_filter_css_datauri_image', function($base64array, $path) {
+ $head = 'data:image/png;base64,';
+ $data = 'iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAQAAAC1HAwCAAAAC0lEQVR42mNkYAAAAAYAAjCB0C8AAAAASUVORK5CYII=';
+
+ $result['full'] = $head . $data;
+ $result['base64data'] = $data;
+ return $result;
+ }, 10, 2);
+
+ $instance = new autoptimizeStyles($css_orig);
+ $instance->setOption('datauris', true);
+ $css_actual = $instance->rewrite_assets($css_orig);
+ $this->assertEquals($css_expected, $css_actual);
+ }
}

4 comments on commit a39edcb

futtta commented on a39edcb Mar 8, 2017

what's the advantage of "sort replacements array by key length in desc order" @zytzagoo

Owner

zytzagoo replied Mar 8, 2017

If/when css contains something like this:

.something-else {background:url(wp-content/themes/my-theme/images/shadow.png) -100px 0 repeat-y;}
.another-thing { background:url(/wp-content/themes/my-theme/images/shadow.png) 0 -767px repeat-x; }

then, when fixurls()/rewrite_assets() does replacements (without sorting), since the replacements array is keyed on the url, the first key is: wp-content/themes/my-theme/images/shadow.png, and since we're using str_replace() (which replaces ALL occurences), that key/string exists in two places now, and both are replaced.

That's a problem when we're replacing these urls with base64 strings, because it results in the second css rule having a / in the beginning (followed by the base64 string), which is wrong.
i.e., it ends up looking like this:

.something-else {background:url($base64) -100px 0 repeat-y;}
.another-thing { background:url(/$base64) 0 -767px repeat-x; }

When the array is sorted, the longest key is searched/replaced first, and the longer key/url (in this case the root-relative one [starts with /]) is fully/properly replaced first, and then the second one too, and in the end, we get:

.something-else {background:url($base64) -100px 0 repeat-y;}
.another-thing { background:url($base64) 0 -767px repeat-x; }

So, to answer your question, the advantage is that it works properly if/when the code calls rewrite_assets() without calling fixurls() first (or vice/versa, kind of).

I noticed it while working on testing the base64 replacements (and trying to call minimal amounts of code to get/test results needed to verify everything is ok).

It doesn't always manifest itself in the wild, because fixurls() is usually called before rewrite_assets(), and that kind of "normalizes" the above two urls into their protocol-relative variants (//example.org/wp-content/themes/my-theme/images/shadow.png), which are then "easily" replaced everywhere.

But it's very tight coupling of code, which stinks and makes reasoning about things way harder than it needs to be -- IMO, in the end, rewrite_assets() should not really rely on something else being previously called (and if fixurls() is required for it to do it's job, it should be done within rewrite_assets() then, or something to that effect).

OK, I see ... Although I'm sure you're right (I see the tight coupling and understand the risk), this to me is "fixing what isn't broken" (as indeed fixurls always gets called before rewriting). Given AO's current install-base I have no choice but to be very conservative, so I'm just going to remove the MHTML-stuff and be done with it.

Owner

zytzagoo replied Mar 8, 2017

Yeah, I understand.

If/when you go with the latest ASSETS_REGEX variant, don't forget to do 2e9f1ff#diff-96087a310ce2ff9d4c8f862e55a9bd62R741 and b4376e9#diff-96087a310ce2ff9d4c8f862e55a9bd62R775 among other things, or stuff could still get lost (refering to upstream #87).

Please sign in to comment.