Fix "CSSMin url() value remapping not working in certain obscure cases"

This patch extends the regex used to find URLs by css block quote subpatterns.

Bug: 60077
Change-Id: Ife00d01b78c6369f20ffc49d20aa58e354a61ee7

diff --git a/includes/libs/CSSMin.php b/includes/libs/CSSMin.php
index 10277e6..605e094 100644 (file)
--- a/includes/libs/CSSMin.php
+++ b/includes/libs/CSSMin.php
@@ -203,11 +203,12 @@ class CSSMin {
                        $remote = substr( $remote, 0, -1 );
                }
 
-               // Note: This will not correctly handle cases where ';', '{' or '}'
-               // appears in the rule itself, e.g. in a quoted string. You are advised
+               // Note: This will correctly handle cases where ';', '{' or '}' appears
+               // in a css block comment (/* ... */). All other occurrences, e.g. in
+               // quoted strings, will not be handled correctly. You are advised
                // not to use such characters in file names. We also match start/end of
                // the string to be consistent in edge-cases ('@import url(…)').
-               $pattern = '/(?:^|[;{])\K[^;{}]*' . CSSMin::URL_REGEX . '[^;}]*(?=[;}]|$)/';
+               $pattern = '/(?:^|[;{])\K(?:\/\*.*?\*\/|[^;{}])*' . CSSMin::URL_REGEX . '(?:\/\*.*?\*\/|[^;}])*(?=[;}]|$)/';
 
                return preg_replace_callback(
                        $pattern,

diff --git a/tests/phpunit/includes/libs/CSSMinTest.php b/tests/phpunit/includes/libs/CSSMinTest.php
index bb5e398..a6f7914 100644 (file)
--- a/tests/phpunit/includes/libs/CSSMinTest.php
+++ b/tests/phpunit/includes/libs/CSSMinTest.php
@@ -291,6 +291,41 @@ class CSSMinTest extends MediaWikiTestCase {
                                '@import url(//localhost/styles.css?query=yes)',
                                '@import url(//localhost/styles.css?query=yes)',
                        ),
+                       array(
+                               'Simple case with comments before url',
+                               'foo { prop: /* some {funny;} comment */ url(bar.png); }',
+                               'foo { prop: /* some {funny;} comment */ url(http://localhost/w/bar.png); }',
+                       ),
+                       array(
+                               'Simple case with comments after url',
+                               'foo { prop: url(red.gif)/* some {funny;} comment */ ; }',
+                               'foo { prop: url(http://localhost/w/red.gif?timestamp)/* some {funny;} comment */ ; }',
+                       ),
+                       array(
+                               'Embedded file with comment before url',
+                               'foo { /* @embed */ background: /* some {funny;} comment */ url(red.gif); }',
+                               "foo { background: /* some {funny;} comment */ url($red); background: /* some {funny;} comment */ url(http://localhost/w/red.gif?timestamp)!ie; }",
+                       ),
+                       array(
+                               'Embedded file with comment after url',
+                               'foo { /* @embed */ background: url(red.gif) /* some {funny;} comment */; }',
+                               "foo { background: url($red) /* some {funny;} comment */; background: url(http://localhost/w/red.gif?timestamp) /* some {funny;} comment */!ie; }",
+                       ),
+                       array(
+                               'Embedded file with comment outside the rule',
+                               'foo { /* @embed */ background: url(red.gif); /* some {funny;} comment */ }',
+                               "foo { background: url($red); background: url(http://localhost/w/red.gif?timestamp)!ie; /* some {funny;} comment */ }",
+                       ),
+                       array(
+                               'Rule with two urls, each with comments',
+                               '{ background: /*asd*/ url(something.png); background: /*jkl*/ url(something.png); }',
+                               '{ background: /*asd*/ url(http://localhost/w/something.png); background: /*jkl*/ url(http://localhost/w/something.png); }',
+                       ),
+                       array(
+                               'Sanity check for offending line from jquery.ui.theme.css (bug 60077)',
+                               '.ui-state-default, .ui-widget-content .ui-state-default, .ui-widget-header .ui-state-default { border: 1px solid #d3d3d3/*{borderColorDefault}*/; background: #e6e6e6/*{bgColorDefault}*/ url(images/ui-bg_glass_75_e6e6e6_1x400.png)/*{bgImgUrlDefault}*/ 50%/*{bgDefaultXPos}*/ 50%/*{bgDefaultYPos}*/ repeat-x/*{bgDefaultRepeat}*/; font-weight: normal/*{fwDefault}*/; color: #555555/*{fcDefault}*/; }',
+                               '.ui-state-default, .ui-widget-content .ui-state-default, .ui-widget-header .ui-state-default { border: 1px solid #d3d3d3/*{borderColorDefault}*/; background: #e6e6e6/*{bgColorDefault}*/ url(http://localhost/w/images/ui-bg_glass_75_e6e6e6_1x400.png)/*{bgImgUrlDefault}*/ 50%/*{bgDefaultXPos}*/ 50%/*{bgDefaultYPos}*/ repeat-x/*{bgDefaultRepeat}*/; font-weight: normal/*{fwDefault}*/; color: #555555/*{fcDefault}*/; }',
+                       ),
                );
        }