lautis / uglifier

Ruby wrapper for UglifyJS JavaScript compressor.
http://www.rubydoc.info/gems/uglifier
MIT License
614 stars 81 forks source link

Broken JS minifing pdf.js #47

Closed andreacfm closed 10 years ago

andreacfm commented 11 years ago

Attempting to minify mozilla/pdf.js brakes js. ( see mozilla/pdf.js#2479) Using the file I get this error:

SyntaxError: octal literals and octal escape sequences are deprecated [Break On This Error]

...-b);var w=n.yMax||y,E=-n.yMin||-b;return"\0$ô\0\0\0Š»\0\0\0ŒŠ»\0\0ß\01...

Compression is made on rails asset pipeline.

lautis commented 11 years ago

Works for me with Uglifier 2.0.0.

phillbaker commented 11 years ago

As a follow up to this, @lautis can you clarify if you used any specific options that allowed you to compile and then use pdf.js?

The specific issue I get (same as above) is that uglifier 2.1.0, with default options compiles

// This is one example of problematic encoding currently around L20107 of https://github.com/mozilla/pdf.js/blob/gh-pages/build/pdf.js
    return '\x00\x03' + // version
           '\x02\x24' + // xAvgCharWidth
           '\x01\xF4' + // usWeightClass
           '\x00\x05' + // usWidthClass
           '\x00\x00' + // fstype (0 to let the font loads via font-face on IE)
           '\x02\x8A' + // ySubscriptXSize
           '\x02\xBB' + // ySubscriptYSize
           '\x00\x00' + // ySubscriptXOffset
           '\x00\x8C' + // ySubscriptYOffset
           '\x02\x8A' + // ySuperScriptXSize
           '\x02\xBB' + // ySuperScriptYSize
           '\x00\x00' + // ySuperScriptXOffset
           '\x01\xDF' + // ySuperScriptYOffset
           '\x00\x31' + // yStrikeOutSize
           '\x01\x02' + // yStrikeOutPosition
           '\x00\x00' + // sFamilyClass
           '\x00\x00\x06' +
           String.fromCharCode(properties.fixedPitch ? 0x09 : 0x00) +
           '\x00\x00\x00\x00\x00\x00' + // Panose
           string32(ulUnicodeRange1) + // ulUnicodeRange1 (Bits 0-31)
           string32(ulUnicodeRange2) + // ulUnicodeRange2 (Bits 32-63)
           string32(ulUnicodeRange3) + // ulUnicodeRange3 (Bits 64-95)
           string32(ulUnicodeRange4) + // ulUnicodeRange4 (Bits 96-127)
           '\x2A\x32\x31\x2A' + // achVendID
           string16(properties.italicAngle ? 1 : 0) + // fsSelection
           string16(firstCharIndex ||
                    properties.firstChar) + // usFirstCharIndex
           string16(lastCharIndex || properties.lastChar) +  // usLastCharIndex
           string16(typoAscent) + // sTypoAscender
           string16(typoDescent) + // sTypoDescender
           '\x00\x64' + // sTypoLineGap (7%-10% of the unitsPerEM value)
           string16(winAscent) + // usWinAscent
           string16(winDescent) + // usWinDescent
           '\x00\x00\x00\x00' + // ulCodePageRange1 (Bits 0-31)
           '\x00\x00\x00\x00' + // ulCodePageRange2 (Bits 32-63)
           string16(properties.xHeight) + // sxHeight
           string16(properties.capHeight) + // sCapHeight
           string16(0) + // usDefaultChar
           string16(firstCharIndex || properties.firstChar) + // usBreakChar
           '\x00\x03';  // usMaxContext

to

return"\0$ô\0\0\0Š»\0\0\0ŒŠ»\0\0ß\01\0\0\0\0"+String.fromCharCode(e.fixedPitch?9:0)+"\0\0\0\0\0\0"+d(i)+d(n)+d(c)+d(s)+"*21*"+h(e.italicAngle?1:0)+h(o||e.firstChar)+h(f||e.lastChar)+h(b)+h(y)+"\0d"+h(k)+h(w)+"\0\0\0\0"+"\0\0\0\0"+h(e.xHeight)+h(e.capHeight)+h(0)+h(o||e.firstChar)+"\0"

This seems to be an encoding/escaping issue. So I tried with { :output => { :ascii_only => true } }, which generates for the same line

return"\0$\xf4\0\0\0\x8a\xbb\0\0\0\x8c\x8a\xbb\0\0\xdf\01\0\0\0\0"+String.fromCharCode(e.fixedPitch?9:0)+"\0\0\0\0\0\0"+d(i)+d(n)+d(c)+d(s)+"*21*"+h(e.italicAngle?1:0)+h(o||e.firstChar)+h(f||e.lastChar)+h(b)+h(y)+"\0d"+h(k)+h(w)+"\0\0\0\0"+"\0\0\0\0"+h(e.xHeight)+h(e.capHeight)+h(0)+h(o||e.firstChar)+"\0"

Both of these generated js files throw "Error: SyntaxError: octal literals and octal escape sequences are deprecated" when used.

Any help would be appreciated.

CC https://github.com/mozilla/pdf.js/issues/2479

lautis commented 11 years ago

You're right, even the latest uglifier seems to result in this behaviour. I misunderstood the description or managed to try this out with a JS interpreter that ignores strict mode. Anyway, I've opened a PR for this bug in UglifyJS2.

phillbaker commented 11 years ago

Thanks @lautis.

For reference, I'd also point out: https://github.com/ndbroadbent/turbo-sprockets-rails3/pull/64#issuecomment-18654913, which is a solution to Uglify compiling CodeMirror, however, in that case { :output => { :ascii_only => true } } does work. It looks like the difference might be hexadecimal escape characters vs. octal?

lautis commented 11 years ago

I had similar issues with characters outside Basic Multilingual Plane using Node 0.4 as JavaScript runtime. Newer Nodes can handle 4-byte UTF-8, but I haven't checked out if those still get corrupted and where the corruption happens.

lautis commented 11 years ago

Fixed in 2.2.0.

phillbaker commented 11 years ago

Awesome! Much appreciated.

andreacfm commented 11 years ago

Is not solved from my side. Even with 2.2.1 I get

SyntaxError: invalid property id
..."ۇٴ",ٸ:"يٴ",ำ:"ํา",ຳ:"ໍາ",ໜ:"ຫນ",ໝ:"ຫມ",ཷ:"ྲཱྀ",ཹ:"ླཱྀ",ẚ:"aʾ","᾽":" ̓","᾿":" ̓","...
brendandc commented 10 years ago

I hit the same issue with 2.2.1, running jruby 1.7.4 on windows, rails 4, asset pipeline and uglifier as the compressor. I'm curious what your environment is @andreacfm , maybe there's a similarity there with us vs. what others are doing..

My content is also in a js object, with keys/values like: "プリウス" : "トヨタ", "アクア" : "トヨタ", "ヴィッツ" : "トヨタ",

lautis commented 10 years ago

Sorry, I dont' have a Windows box to test so I can't verify this. There could be issues with non-ASCII characters and Windows Script Host. Installing therubyracer or therubyrhino could help.

fwoeck commented 10 years ago

I see the same effects as @andreacfm - with an ubuntu node.js v0.10.18 and the uglifier gem 2.2.1. For reference here is the uncompressed pdf.worker.js and the uglified version:

http://dokmatic-cdn.s3.amazonaws.com/pdf.worker.js http://dokmatic-cdn.s3.amazonaws.com/pdf.worker-ae4b62e34ddaa13ac163e8ff46f487fb.js

the errors stem from the lines around #16500 of the original.

andreacfm commented 10 years ago

My issue happens on ubuntu and osx. I did not test on win. Il giorno 21/set/2013 17:19, "Frank Wöckener" notifications@github.com ha scritto:

I see the same effects as @andreacfm https://github.com/andreacfm - with an ubuntu node.js v0.10.18 and the uglifier gem 2.2.1. For reference here is the uncompressed pdf.worker.js and the uglified version:

http://dokmatic-cdn.s3.amazonaws.com/pdf.worker.js

http://dokmatic-cdn.s3.amazonaws.com/pdf.worker-ae4b62e34ddaa13ac163e8ff46f487fb.js

the errors stem from the lines around #16500 of the original.

— Reply to this email directly or view it on GitHubhttps://github.com/lautis/uglifier/issues/47#issuecomment-24864252 .

lautis commented 10 years ago

Thanks for the sample files. There seems to be an issue with object keys in UglifyJS. I'll update Uglifier when UglifyJS has a fix for this.

As a workaround, you could try to use the quote_keys output option, Uglifier.new(output: {quote_keys: true}). This at least produces valid output.

jasonwebster commented 10 years ago

Is there an issue filed with them for this? I looked and couldn't find anything.

lautis commented 10 years ago

I believe this has been addressed in mishoo/UglifyJS2#308. I've pulled the latest UglifyJS master to Uglifier, this should fix the issues with Unicode keys.

andreacfm commented 10 years ago

When do you think the changes will be pushed to rubygems?

Thanks

On Oct 19, 2013, at 6:26 PM, Ville Lautanala notifications@github.com wrote:

I believe this has been addressed in mishoo/UglifyJS2#308. I've pulled the latest UglifyJS master to Uglifier, this should fix the issues with Unicode keys.

— Reply to this email directly or view it on GitHub.

Andrea Campolonghi acampolonghi@gmail.com

lautis commented 10 years ago

I just pushed 2.3.0 to rubygems.

andreacfm commented 10 years ago

Thanks

Ville Lautanala wrote:

I just pushed 2.3.0 to rubygems.

— Reply to this email directly or view it on GitHub https://github.com/lautis/uglifier/issues/47#issuecomment-27143476.