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

Comments

Projects
None yet
3 participants
@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:

  • sometime compressor is not desireable
  • i have to set unused: false due #321

Is it possible to fix?

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

@mishoo

This comment has been minimized.

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

This comment has been minimized.

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

This comment has been minimized.

Copy link
Collaborator

rvanvelzen commented Oct 25, 2013

What version of Safari are you running?

@puzrin

This comment has been minimized.

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

This comment has been minimized.

Copy link
Collaborator

rvanvelzen commented Oct 25, 2013

@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

Fix mangling issues with safari
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

This comment has been minimized.

Copy link
Collaborator

rvanvelzen commented Oct 25, 2013

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?

mishoo added a commit that referenced this issue Oct 29, 2013

@mishoo mishoo closed this Oct 29, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.