Last modified: 2012-04-16 09:15:37 UTC
Found this while tidying up work on SVGEdit; ext.svgedit.embedapi.js get incorrectly minified by the new JSDistiller stuff. This file is a short bit of (rather messy and comment-filled ;) third-party JS which defines functions for communicated with an embedded iframe. I found I got a syntax error at this point in the original file: var t = this //new callback for(var g = 0, args = []; g < arguments.length; g++){ args.push(arguments[g]); } which gets minified to: var t=thisfor(var g=0,args=[];g<arguments.length;g++){ args.push(arguments[g]); } The first line ('var t = this') allows the statement terminator to be implied rather than using an explicit semicolon -- a poor practice, but legitimate in JS. It also uses a double-slash single-line comment on the end. The combination of these two seems to confuse the new minifier, and it ends up slapping them together with neither a semicolon nor any whitespace at all! It needs to either keep the line feed intact as a statement terminator, or add a semicolon. I'll work around it for now by adjusting the formatting in this particular file. Adding to MW 1.17 blockers (unless we won't be shipping this version of the minifier -- the old code did fine)
We need minimifier tests.
(In reply to comment #0) > The combination of these two seems to confuse the new minifier, and it ends up > slapping them together with neither a semicolon nor any whitespace at all! > The real bug here is that the newline after a C++-style // comment is stripped unconditionally, even though the minifier keeps each statement on its own line unless $wgResourceLoaderMinifyJSVerticalSpace is set to true. I have changed it to unconditionally *preserve* such newlines in r81246, and confirmed that this fixes your test case. > Adding to MW 1.17 blockers (unless we won't be shipping this version of the > minifier -- the old code did fine) The fix has been tagged for 1.17 merging.