Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ie8 and mangle can result in incorrect Javascript code in the presence of try...except blocks #3215

Closed
jfly opened this issue Jul 17, 2018 · 2 comments · Fixed by #3216
Closed
Labels

Comments

@jfly
Copy link

jfly commented Jul 17, 2018

Bug report

Uglify version (uglifyjs -V)
3.4.5

JavaScript input

function foo() {
    var createEvent = function createEvent(name) {
        return "hiya";
    };
    try {
        new Event('test');
    } catch (e) {
        createEvent = function createEvent(name) {
            return "biya";
        };
    }
    console.log(createEvent);
}

The uglifyjs CLI command executed or minify() options used.
--ie8 --mangle

JavaScript output.

$ npx uglify-js@3.4.5 ./autosize.js --ie8 --mangle | npx prettier --stdin-filepath autosize.js
npx: installed 3 in 0.796s
npx: installed 1 in 0.853s
function foo() {
  var n = function n(n) {
    return "hiya";
  };
  try {
    new Event("test");
  } catch (t) {
    createEvent = function createEvent(n) {
      return "biya";
    };
  }
  console.log(n);
}

Note how the produced javascript code sets a createEvent variable in the catch above, despite the fact that createEvent was renamed to n in the other two places where the original Javascript referred to it. This is incorrect behavior. I would expect the two occurrences of createEvent in the catch to also be changed to n.

The above code may look kind of weird, but it's a simplified version of this real life code from autosize: https://github.com/jackmoore/autosize/blob/6ae70cd3d28888eb858eef1167e6ed75d25dff7f/dist/autosize.js#L48-L60. See thewca/worldcubeassociation.org#3047 for more information about how we ran into this issue.

@alexlamsl
Copy link
Collaborator

@jfly thanks for the detailed report - fix is on the way.

alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Jul 17, 2018
@jfly
Copy link
Author

jfly commented Jul 17, 2018

Whooo! Thanks for the quick response!

alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Jul 17, 2018
alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Jul 17, 2018
alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Jul 17, 2018
alexlamsl added a commit to alexlamsl/UglifyJS that referenced this issue Jul 18, 2018
alexlamsl added a commit that referenced this issue Jul 19, 2018
jfly added a commit to jfly/worldcubeassociation.org that referenced this issue Aug 7, 2018
This reverts commit 9720cae.

This is safe to do because we upgraded uglifier in
71204b6 to 4.1.17, which brings along
uglifyjs 3.4.6, which includes a fix for
mishoo/UglifyJS#3215.

This fixes thewca#3047.
jonatanklosko pushed a commit to thewca/worldcubeassociation.org that referenced this issue Aug 13, 2018
This reverts commit 9720cae.

This is safe to do because we upgraded uglifier in
71204b6 to 4.1.17, which brings along
uglifyjs 3.4.6, which includes a fix for
mishoo/UglifyJS#3215.

This fixes #3047.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants