From bf3b97791bd790948de1cba6671330d8e496aca0 Mon Sep 17 00:00:00 2001 From: =?utf8?q?Bartosz=20Dziewo=C5=84ski?= Date: Mon, 22 Sep 2014 14:21:47 +0200 Subject: [PATCH] CSSMin: Don't generate double rules for IE < 8 when embedding SVG files Bug: 71003 Change-Id: Ic232e8d62b164940003afdfe7cce9f964d7e9cbc --- RELEASE-NOTES-1.25 | 2 + includes/libs/CSSMin.php | 81 ++++++++++++++++------ tests/phpunit/includes/libs/CSSMinTest.php | 5 +- 3 files changed, 64 insertions(+), 24 deletions(-) diff --git a/RELEASE-NOTES-1.25 b/RELEASE-NOTES-1.25 index d05a61fe7c..e5a6cd1a1b 100644 --- a/RELEASE-NOTES-1.25 +++ b/RELEASE-NOTES-1.25 @@ -20,6 +20,8 @@ production. percent-encoding), but up to 20% decrease after it. === Bug fixes in 1.25 === +* (bug 71003) No additional code will be generated to try to load CSS-embedded + SVG images in Internet Explorer 6 and 7, as they don't support them anyway. === Action API changes in 1.25 === * (bug 65403) XML tag highlighting is now only performed for formats diff --git a/includes/libs/CSSMin.php b/includes/libs/CSSMin.php index 91f2c61e32..6eb5258d33 100644 --- a/includes/libs/CSSMin.php +++ b/includes/libs/CSSMin.php @@ -265,9 +265,12 @@ class CSSMin { ); if ( $embedData ) { + // Remember the occurring MIME types to avoid fallbacks when embedding some files. + $mimeTypes = array(); + $ruleWithEmbedded = preg_replace_callback( $pattern, - function ( $match ) use ( $embedAll, $local, $remote ) { + function ( $match ) use ( $embedAll, $local, $remote, &$mimeTypes ) { $embed = $embedAll || $match['embed']; $embedded = CSSMin::remapOne( $match['file'], @@ -277,21 +280,35 @@ class CSSMin { $embed ); + $url = $match['file'] . $match['query']; + $file = $local . $match['file']; + if ( + !CSSMin::isRemoteUrl( $url ) && !CSSMin::isLocalUrl( $url ) + && file_exists( $file ) + ) { + $mimeTypes[ CSSMin::getMimeType( $file ) ] = true; + } + return CSSMin::buildUrlValue( $embedded ); }, $rule ); + + // Are all referenced images SVGs? + $needsEmbedFallback = $mimeTypes !== array( 'image/svg+xml' => true ); } - if ( $embedData && $ruleWithEmbedded !== $ruleWithRemapped ) { - // Build 2 CSS properties; one which uses a base64 encoded data URI in place - // of the @embed comment to try and retain line-number integrity, and the - // other with a remapped an versioned URL and an Internet Explorer hack + if ( !$embedData || $ruleWithEmbedded === $ruleWithRemapped ) { + // We're not embedding anything, or we tried to but the file is not embeddable + return $ruleWithRemapped; + } elseif ( $embedData && $needsEmbedFallback ) { + // Build 2 CSS properties; one which uses a data URI in place of the @embed comment, and + // the other with a remapped and versioned URL with an Internet Explorer 6 and 7 hack // making it ignored in all browsers that support data URIs return "$ruleWithEmbedded;$ruleWithRemapped!ie"; } else { - // No reason to repeat twice - return $ruleWithRemapped; + // Look ma, no fallbacks! This is for files which IE 6 and 7 don't support anyway: SVG. + return $ruleWithEmbedded; } }, $source ); @@ -305,6 +322,34 @@ class CSSMin { } + /** + * Is this CSS rule referencing a remote URL? + * + * @private Until we require PHP 5.5 and we can access self:: from closures. + * @param string $maybeUrl + * @return bool + */ + public static function isRemoteUrl( $maybeUrl ) { + if ( substr( $maybeUrl, 0, 2 ) === '//' || parse_url( $maybeUrl, PHP_URL_SCHEME ) ) { + return true; + } + return false; + } + + /** + * Is this CSS rule referencing a local URL? + * + * @private Until we require PHP 5.5 and we can access self:: from closures. + * @param string $maybeUrl + * @return bool + */ + public static function isLocalUrl( $maybeUrl ) { + if ( !self::isRemoteUrl( $maybeUrl ) && $maybeUrl !== '' && $maybeUrl[0] === '/' ) { + return true; + } + return false; + } + /** * Remap or embed a CSS URL path. * @@ -319,22 +364,16 @@ class CSSMin { // The full URL possibly with query, as passed to the 'url()' value in CSS $url = $file . $query; - // Skip fully-qualified and protocol-relative URLs and data URIs - if ( substr( $url, 0, 2 ) === '//' || parse_url( $url, PHP_URL_SCHEME ) ) { - return $url; + // Expand local URLs with absolute paths like /w/index.php to possibly protocol-relative URL, if + // wfExpandUrl() is available. (This will not be the case if we're running outside of MW.) + if ( self::isLocalUrl( $url ) && function_exists( 'wfExpandUrl' ) ) { + return wfExpandUrl( $url, PROTO_RELATIVE ); } - // URLs with absolute paths like /w/index.php need to be expanded - // to absolute URLs but otherwise left alone - if ( $url !== '' && $url[0] === '/' ) { - // Replace the file path with an expanded (possibly protocol-relative) URL - // ...but only if wfExpandUrl() is even available. - // This will not be the case if we're running outside of MW - if ( function_exists( 'wfExpandUrl' ) ) { - return wfExpandUrl( $url, PROTO_RELATIVE ); - } else { - return $url; - } + // Pass thru fully-qualified and protocol-relative URLs and data URIs, as well as local URLs if + // we can't expand them. + if ( self::isRemoteUrl( $url ) || self::isLocalUrl( $url ) ) { + return $url; } if ( $local === false ) { diff --git a/tests/phpunit/includes/libs/CSSMinTest.php b/tests/phpunit/includes/libs/CSSMinTest.php index 4a1885af89..6fa3acf81f 100644 --- a/tests/phpunit/includes/libs/CSSMinTest.php +++ b/tests/phpunit/includes/libs/CSSMinTest.php @@ -237,10 +237,9 @@ class CSSMinTest extends MediaWikiTestCase { "foo { background: url(http://localhost/w/large.png?timestamp); }", ), array( - 'SVG files are embedded without base64 encoding', + 'SVG files are embedded without base64 encoding and unnecessary IE 6 and 7 fallback', 'foo { /* @embed */ background: url(circle.svg); }', - "foo { background: url($svg); " - . "background: url(http://localhost/w/circle.svg?timestamp)!ie; }", + "foo { background: url($svg); }", ), array( 'Two regular files in one rule', -- 2.20.1