From b047e061445e4eb4f6d142cad79c19745d8b6143 Mon Sep 17 00:00:00 2001 From: Tim Otten Date: Wed, 8 Apr 2015 22:28:53 -0700 Subject: [PATCH] CRM-16246, CRM-16145 - When aggregating JS, strip full-line "//" comments. In discussion of CRM-16246, we found that aggregation led to errors in processing "//# sourceMappingURL=" comments in Chrome. We can't fix that for all aggregators, but we can do something about the internal aggregator. --- CRM/Utils/JS.php | 19 ++++++++++++++++ Civi/Angular/Page/Modules.php | 3 ++- tests/phpunit/CRM/Utils/JSTest.php | 35 ++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 1 deletion(-) diff --git a/CRM/Utils/JS.php b/CRM/Utils/JS.php index c006cf8ea2..63d6d85d79 100644 --- a/CRM/Utils/JS.php +++ b/CRM/Utils/JS.php @@ -108,4 +108,23 @@ class CRM_Utils_JS { return $scripts; } + /** + * This is a primitive comment stripper. It doesn't catch all comments + * and falls short of minification, but it doesn't munge Angular injections + * and is fast enough to run synchronously (without caching). + * + * At time of writing, running this against the Angular modules, this impl + * of stripComments currently adds 10-20ms and cuts ~7%. + * + * Please be extremely cautious about extending this. If you want better + * minification, you should probably remove this implementation, + * import a proper JSMin implementation, and cache its output. + * + * @param string $script + * @return string + */ + public static function stripComments($script) { + return preg_replace(":^\\s*//[^\n]+$:m", "", $script); + } + } diff --git a/Civi/Angular/Page/Modules.php b/Civi/Angular/Page/Modules.php index 0cd9744a5c..0190895d30 100644 --- a/Civi/Angular/Page/Modules.php +++ b/Civi/Angular/Page/Modules.php @@ -74,7 +74,8 @@ class Modules extends \CRM_Core_Page { array('angular', '$', '_'), array('angular', 'CRM.$', 'CRM._') ); - return implode("\n", $scripts); + // This impl of stripComments currently adds 10-20ms and cuts ~7% + return \CRM_Utils_JS::stripComments(implode("\n", $scripts)); } /** diff --git a/tests/phpunit/CRM/Utils/JSTest.php b/tests/phpunit/CRM/Utils/JSTest.php index 05d2487ff9..9696d869a8 100644 --- a/tests/phpunit/CRM/Utils/JSTest.php +++ b/tests/phpunit/CRM/Utils/JSTest.php @@ -153,4 +153,39 @@ class CRM_Utils_JSTest extends CiviUnitTestCase { ); $this->assertEquals($expectedOutput, implode("", $actualOutput)); } + + public function stripCommentsExamples() { + $cases = array(); + $cases[] = array( + "a();\n//# sourceMappingURL=../foo/bar/baz.js\nb();", + "a();\n\nb();", + ); + $cases[] = array( + "// foo\na();", + "\na();", + ); + $cases[] = array( + "b();\n // foo", + "b();\n", + ); + $cases[] = array( + "/// foo\na();\n\t \t//bar\nb();\n// whiz", + "\na();\n\nb();\n", + ); + $cases[] = array( + "alert('//# sourceMappingURL=../foo/bar/baz.js');\n//zoop\na();", + "alert('//# sourceMappingURL=../foo/bar/baz.js');\n\na();", + ); + return $cases; + } + + /** + * @param string $input + * @param string $expectedOutput + * @dataProvider stripCommentsExamples + */ + public function testStripComments($input, $expectedOutput) { + $this->assertEquals($expectedOutput, CRM_Utils_JS::stripComments($input)); + } + } -- 2.25.1