mangle bug with eval
#3768
Comments
|
Thanks for the report & fix − learnt something new about |
|
Ah, so it was just a case of giving up due to the many peculiar behaviour of $ cat test.js
console.log(function() {
var a = 42;
return eval("typeof a");
}(), function(e) {
var a = null;
return e("typeof a");
}(eval), function(eval) {
var a = false;
return eval("typeof a");
}(eval), function(f) {
var a = "STRING";
var eval = f;
return eval("typeof a");
}(eval));
$ cat test.js | node
number undefined boolean stringThat wiki link would be more helpful if it also specifies the holding variable name cannot be |
|
Since you've specified |
|
It appears the current implementation is actually accurate, nonetheless let's add these test cases. |
|
Although the Node results are not conclusive as you pointed out: if evalbug is run in the browser it produces "PASS": I think the browser behavior is what we want to preserve. |
|
In that browser case, you are putting the code on the root level, so you can't mangle with |
|
I don't see why one could not mangle with toplevel true. All the symbol names within the purview of |
|
There is a huge downside, namely it disables a lot of In the browser case above, one can do: <!DOCTYPE html>
<html>
<body>
<script>
var x = "PASS";
(function() {
alert(x);
})();
</script>
<h1>Hello World!</h1>
<script>
alert(x);
</script>
</body>
</html>No |
|
The example above should still work with a name cache with toplevel mangle if the two script sections were independently minified. For that matter,
I'm arguing for logical consistency between Think of the patch as a safeguard when used with |
As shown in #3768 (comment), the scope which
In your second example you have invoked I still fail to see any inconsistencies − yes (Will have a look at the |
|
Look at it this way − when you specified (function() {
var e = eval, x = "PASS";
(function() {
console.log(e("x"));
})();
})();Now, that |
|
How about this − if we see Would that address your concern about separation of |
evalbug.html shows the issue quite clearly. If the code is mangled with toplevel with uglify-js it will not work in the browser. It ought to. None of the examples you have shown refute that, and the fix I proposed would be compatible with your new tests if I guess it's pointless to debate this further. I'll use the fix in my fork. |
Given:
Expected:
Actual:
The fix:
With fix:
The text was updated successfully, but these errors were encountered: