Using the JSMIN+ option works well but when I enable the JSMIN php extension then I get javascript errors. I have discovered that this is due to the scripts being compressed in a different order than with the JSMin+ option.
I believe that the call to jsmin() is happening too early. If I enable the JSMin+ option and then make the following code change, replacing the JSMinPlus call with a call to jsmin() then everything is fine. There is much logic that is happening before this which isn't happening before the real call to jsmin() so clearly the fault is not with the JSMIN php extension but with the advagg_js_compress module.
Index: advagg_js_compress/advagg_js_compress.module
===================================================================
--- advagg_js_compress/advagg_js_compress.module (revision 1218)
+++ advagg_js_compress/advagg_js_compress.module (working copy)
@@ -289,11 +289,12 @@
try {
$contents = str_replace(pack("CCC", 0xef, 0xbb, 0xbf), "", $contents);
ob_start();
- $contents = JSMinPlus::minify($contents);
- $error = trim(ob_get_contents());
- if (!empty($error)) {
- throw new Exception($error);
- }
+ $contents = jsmin($contents);
$after = strlen($contents);
}
catch (Exception $e) {
Comments
Comment #1
tangent commentedI may have been wrong about the file order. I suspect my issue is caused by BOMs in the source files. #538274: Aggregated javascript contains BOM's and thereby breaks JSMIN+ says that JSMIN should handle this even though JSMIN+ does not. Is there a regression in JSMIN perhaps?
Copying the BOM stripping code before the jsmin() call resolves the issue for me.
Comment #2
mikeytown2 commentedThanks for finding this bug! Here's a patch for you to review.
Comment #3
tangent commentedDisregard my previous comment. JSMIN does not work even with BOM stripping. I was probably see a boost cached page or something when it seemed like it was working. The $contents variable is definitely getting rebuilt when JSMIN+ compression is used, versus JSMIN. The attached patch moves the JSMIN compression closer to where the JSMIN+ compression happens and I get no more errors.
Comment #4
mikeytown2 commentedComment #5
mikeytown2 commentedPatch #3 has been committed. Thanks!