Keep legit code working even when --screw-ie is not passed.
authorMihai Bazon <mihai@bazon.net>
Fri, 22 Mar 2013 16:02:08 +0000 (18:02 +0200)
committerMihai Bazon <mihai@bazon.net>
Fri, 22 Mar 2013 16:04:46 +0000 (18:04 +0200)
Previously:

    Without `--screw-ie`, UglifyJS would always leak names of function
    expressions into the containing scope, as if they were function
    declarations.  That was to emulate IE<9 behavior.  Code relying on this
    IE bug would continue to work properly after mangling, although it would
    only work in IE (since other engines don't share the bug).  Sometimes
    this broke legitimage code (see #153 and #155).

    With `--screw-ie` the names would not be leaked into the current scope,
    working properly in legit cases; but still it broke legit code when
    running in IE<9 (see #24).

Currently:

    Regardless of the `--screw-ie` setting, the names will not be leaked.
    Code relying on the IE bug will not work properly after mangling.
    <evil laughter here>

    Without `--screw-ie`: a hack has been added to the mangler to avoid
    using the same name for a function expression and some other variable in
    the same scope.  This keeps legit code working, at the (negligible,
    indeed) cost of one more identifier.

    With `--screw-ie` you allow the mangler to name function expressions
    with the same identifier as another variable in scope.  After mangling
    code might break in IE<9.

Oh man, the commit message is longer than the patch.

Fix #153, #155

bin/uglifyjs
lib/parse.js
lib/scope.js

index 88fd46b..8a510d6 100755 (executable)
@@ -122,6 +122,10 @@ if (MANGLE && ARGS.r) {
     MANGLE.except = ARGS.r.replace(/^\s+|\s+$/g).split(/\s*,+\s*/);
 }
 
+if (MANGLE && ARGS.screw_ie) {
+    MANGLE.screw_ie = true;
+}
+
 var OUTPUT_OPTIONS = {
     beautify: BEAUTIFY ? true : false
 };
index 5a75e75..dba8702 100644 (file)
@@ -149,7 +149,7 @@ function is_unicode_connector_punctuation(ch) {
 };
 
 function is_identifier(name) {
-    return /^[a-z_$][a-z0-9_$]*$/i.test(name) && !RESERVED_WORDS(name);
+    return !RESERVED_WORDS(name) && /^[a-z_$][a-z0-9_$]*$/i.test(name);
 };
 
 function is_identifier_start(code) {
index c6a8559..503c189 100644 (file)
@@ -62,15 +62,16 @@ SymbolDef.prototype = {
             || (!(options && options.eval) && (this.scope.uses_eval || this.scope.uses_with));
     },
     mangle: function(options) {
-        if (!this.mangled_name && !this.unmangleable(options))
-            this.mangled_name = this.scope.next_mangled(options);
+        if (!this.mangled_name && !this.unmangleable(options)) {
+            var s = this.scope;
+            if (this.orig[0] instanceof AST_SymbolLambda && !options.screw_ie)
+                s = s.parent_scope;
+            this.mangled_name = s.next_mangled(options);
+        }
     }
 };
 
-AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){
-    options = defaults(options, {
-        screw_ie: false
-    });
+AST_Toplevel.DEFMETHOD("figure_out_scope", function(){
     // This does what ast_add_scope did in UglifyJS v1.
     //
     // Part of it could be done at parse time, but it would complicate
@@ -124,15 +125,7 @@ AST_Toplevel.DEFMETHOD("figure_out_scope", function(options){
             node.init_scope_vars();
         }
         if (node instanceof AST_SymbolLambda) {
-            if (options.screw_ie) {
-                scope.def_function(node);
-            } else {
-                // https://github.com/mishoo/UglifyJS2/issues/24 — MSIE
-                // leaks function expression names into the containing
-                // scope.  Don't like this fix but seems we can't do any
-                // better.  IE: please die. Please!
-                (node.scope = scope.parent_scope).def_function(node);
-            }
+            scope.def_function(node);
         }
         else if (node instanceof AST_SymbolDefun) {
             // Careful here, the scope where this should be defined is
@@ -284,14 +277,14 @@ AST_Scope.DEFMETHOD("def_variable", function(symbol){
 });
 
 AST_Scope.DEFMETHOD("next_mangled", function(options){
-    var ext = this.enclosed, n = ext.length;
+    var ext = this.enclosed;
     out: while (true) {
         var m = base54(++this.cname);
         if (!is_identifier(m)) continue; // skip over "do"
         // we must ensure that the mangled name does not shadow a name
         // from some parent scope that is referenced in this or in
         // inner scopes.
-        for (var i = n; --i >= 0;) {
+        for (var i = ext.length; --i >= 0;) {
             var sym = ext[i];
             var name = sym.mangled_name || (sym.unmangleable(options) && sym.name);
             if (m == name) continue out;
@@ -349,7 +342,8 @@ AST_Toplevel.DEFMETHOD("_default_mangler_options", function(options){
         except   : [],
         eval     : false,
         sort     : false,
-        toplevel : false
+        toplevel : false,
+        screw_ie : false
     });
 });