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.

diff --git a/CRM/Utils/JS.php b/CRM/Utils/JS.php
index c006cf8ea201798af106abdcccf70823747f6234..63d6d85d7989603f1bd1b3eff139b80b1e091650 100644 (file)
--- 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 0cd9744a5cbd421bebdccfd59f803adf85b8f6d3..0190895d30dcde012161748275eb88a63d9a4d41 100644 (file)
--- 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 05d2487ff9b88f70814652b95acc7a5e8964ef6e..9696d869a87505a8e3b8c4a3fcb0aadc3e6de1e7 100644 (file)
--- 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));
+  }
+
 }