BarryCarlyon / minify

Automatically exported from code.google.com/p/minify (personal copy)
BSD 3-Clause "New" or "Revised" License
0 stars 0 forks source link

JSMin breaks code like: id++ + or + ++d #144

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
Minify version: 1.1.1
PHP version: 5.2.x

What steps will reproduce the problem?
1. try to minify CKeditor http://ckeditor.com/
2. CKeditor has javscript code somewhere such as "var+ ++" 
3. which is minified into sth. like "var++" and breaks the javascript

Expected output: "+ ++"

Actual output: "++"

Did any unit tests FAIL? (Please do not post the full list)

Please provide any additional information below. If this has to do with URI
rewriting, please include your DOCUMENT_ROOT.

Used in combination with Drupal CMS, CKeditor and javscript aggregator
module which implements JSMin. 

Original issue reported on code.google.com by mkoenra...@gmail.com on 20 Oct 2009 at 3:35

GoogleCodeExporter commented 9 years ago
I see: var x='editor'+ ++l;

This is a bad coding practice that would be flagged by JSLint, but indeed is a 
known 
class of bugs in the JSMin algorithm, and a fix would be quite ugly and 
probably 
painfully slow and brittle. Two options here:

1. add parenthesis to the incremented var: var x='editor'+ (++l);
CKEditor's author might be nice enough to do this for us in a future version, 
but 
I'd understand if he didn't.

2. Serve the file in a group and use a custom source to specify no minifier: 
http://
code.google.com/p/minify/wiki/CustomSource

Original comment by mrclay....@gmail.com on 20 Oct 2009 at 6:03

GoogleCodeExporter commented 9 years ago
Option 3. Switch to the JSMinPlus minifier:
http://code.google.com/p/minify/wiki/CookBook#
(Slightly)_Better_Javascript_Compression
If you have problems with this, please post to the Google Group.

Original comment by mrclay....@gmail.com on 20 Oct 2009 at 6:08

GoogleCodeExporter commented 9 years ago
Found another issue from the same kind : "invalid increment operand"
in ckeditor.js :

(100)+ +(f[2]||0);

When it is minified, it gives (100)++(f[2]||0);

just add parenthesis around "+(f[2]||0)" :

(100)+ (+(f[2]||0));

Original comment by nete...@gmail.com on 26 Oct 2009 at 2:47

GoogleCodeExporter commented 9 years ago
The fix is to remove the completely unnecessary second "+":
(100)+ (f[2]||0);

It's news to me, but apparently this is valid syntax: 1 + - + - + 1; //2

Original comment by mrclay....@gmail.com on 26 Oct 2009 at 3:22

GoogleCodeExporter commented 9 years ago
Issue 168 has been merged into this issue.

Original comment by mrclay....@gmail.com on 8 May 2010 at 7:36

GoogleCodeExporter commented 9 years ago
Issue 175 has been merged into this issue.

Original comment by mrclay....@gmail.com on 8 May 2010 at 7:36

GoogleCodeExporter commented 9 years ago
In R410, as a workaround JSMin::minify will detect syntax like "+ ++" and just 
return 
the JS as-is (assuming it's already been minified). The JSMin.php in R410 is 
safe to 
drop into release 2.1.3.

Can someone verify this works OK? I tried to catch all cases like ++ +, ++ ,- 
++, etc.

Original comment by mrclay....@gmail.com on 8 May 2010 at 7:42

GoogleCodeExporter commented 9 years ago
Thanks.  This fixed it for me.  :) http://newhanoian.com

Original comment by nathanbr...@gmail.com on 21 May 2010 at 9:28

GoogleCodeExporter commented 9 years ago

Original comment by mrclay....@gmail.com on 21 May 2010 at 2:01

GoogleCodeExporter commented 9 years ago
The JSMin just committed to github should now correctly handle this syntax 
(rather than refusing to minify the input):
https://github.com/mrclay/minify/commit/5b6469467ef72496bfdfe74a7d7b0bb33490074f

Please try it out if you can.

Original comment by mrclay....@gmail.com on 26 Jun 2011 at 6:43