Skip to content

Commit

Permalink
Keep legit code working even when --screw-ie is not passed.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mishoo committed Mar 22, 2013
1 parent 24e58ee commit b14d3df
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 19 deletions.
4 changes: 4 additions & 0 deletions bin/uglifyjs
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
Expand Down
2 changes: 1 addition & 1 deletion lib/parse.js
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
30 changes: 12 additions & 18 deletions lib/scope.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -349,7 +342,8 @@ AST_Toplevel.DEFMETHOD("_default_mangler_options", function(options){
except : [],
eval : false,
sort : false,
toplevel : false
toplevel : false,
screw_ie : false
});
});

Expand Down

0 comments on commit b14d3df

Please sign in to comment.