CSSMin: Don't generate double rules for IE < 8 when embedding SVG files

Bug: 71003
Change-Id: Ic232e8d62b164940003afdfe7cce9f964d7e9cbc

diff --git a/RELEASE-NOTES-1.25 b/RELEASE-NOTES-1.25
index d05a61f..e5a6cd1 100644 (file)
--- 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 91f2c61..6eb5258 100644 (file)
--- 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 4a1885a..6fa3acf 100644 (file)
--- 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',