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

Error in Safari after mangling #326

Closed
puzrin opened this issue Oct 24, 2013 · 6 comments
Closed

Error in Safari after mangling #326

puzrin opened this issue Oct 24, 2013 · 6 comments

Comments

@puzrin
Copy link

puzrin commented Oct 24, 2013

Mangler can produce the same name for named function & it's param. That cause error in Safary. Example:

Original:

N.wire.on('cmd:glyph_options', function gopt_dialog(event) {...});

After mangle:

e.wire.on("cmd:glyph_options", function i(i) {...});

That CAN be avoided, if compressor used, but:

Is it possible to fix?

PS. It seems, this bug already was reported in #179. There are more examples.

@mishoo
Copy link
Owner

mishoo commented Oct 24, 2013

The following code does not error out in Safari 6.0.5. Could it be that the issue was fixed?

(function(){
    "use strict";
    function x(x) {
        console.log(x);
    }
    x(10);
})();

I don't like the idea to cripple UglifyJS to work around browser bugs, though that's sometimes necessary. Not sure we should "fix" this one, any more opinions?

@puzrin
Copy link
Author

puzrin commented Oct 24, 2013

If problem is in example - i'll spend time to find a better one. Current situation is, that one of #321 or #326 should be fixed for safe deploy on production sites.

  1. We can't just send all safary users to hell. Safary is popular.
  2. We can't wait 5+ minutes on server start, due slowdown on minified files
  3. We can't expect, that developper will not try to process minified files. Because, in real projects some external vendor modules have no unminified sources. Manual uncompression will be a pain for mainenance.

@rvanvelzen
Copy link
Collaborator

What version of Safari are you running?

@puzrin
Copy link
Author

puzrin commented Oct 25, 2013

Personally, i've tested in Safary 7, OSX. But here is confirmation for 6.1 too fontello/fontello#225 (don't pay anttention on alert on screenshot, it's caused by broken script).

@rvanvelzen
Copy link
Collaborator

@mishoo: I've confirmed that this happens only with anonymous functions.

This will give the aforementioned error:

(function(){
    "use strict";
    (function x(x) {
        console.log(x);
    })(10);
})();

rvanvelzen added a commit to rvanvelzen/UglifyJS2 that referenced this issue Oct 25, 2013
Fixes mishoo#326 and mishoo#179

It isn't a really clean solution, but it works. The idea is to
"virtually" reference the function name inside the scope. But to be able
know it isn't *really* referenced, we add a `virtual` flag to
`AST_SymbolRef.reference`.
@rvanvelzen
Copy link
Collaborator

I've just submitted a pull request with a fix that should do the trick. It isn't nice, but it seems to work. Could you please verify?

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

Successfully merging a pull request may close this issue.

3 participants