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

Build: Fix UglifyJS output in Android 4.0 #3759

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
3 participants
@mgol
Member

mgol commented Aug 16, 2017

Summary

Enabling the ie8 option makes Android 4.0 run tests properly. Before that it
wasn't rendering anything in the QUnit iframe.

Fixes gh-3743

I don't know what exactly causes the issue without the setting. At first sight I only see unescaped throws property and void 0 instead of undefined but just using those two doesn't fail the parsing in Android 4.0: http://jsbin.com/zageloz/edit?html,css,js,console

It'd be worth to investigate longer the real reason for the breakage (perhaps by) but I don't have a lot of time right now. This PR adds 52 bytes to the minified gzipped build which is quite a lot. But even if we don't land this, this PR shows in which direction to look for the real issue.

Checklist

Build: Fix UglifyJS output in Android 4.0
Enabling the ie8 option makes Android 4.0 run tests properly. Before that it
wasn't rendering anything in the QUnit iframe.

Fixes gh-3743

@mgol mgol requested a review from gibson042 Aug 16, 2017

@mention-bot

This comment has been minimized.

Show comment
Hide comment
@mention-bot

mention-bot Aug 16, 2017

@mgol, thanks for your PR! By analyzing the history of the files in this pull request, we identified @markelog, @scottgonzalez and @gibson042 to be potential reviewers.

mention-bot commented Aug 16, 2017

@mgol, thanks for your PR! By analyzing the history of the files in this pull request, we identified @markelog, @scottgonzalez and @gibson042 to be potential reviewers.

mgol referenced this pull request Aug 16, 2017

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Aug 16, 2017

Member

Another option to fix it is to downgrade grunt-contrib-uglify from 3.0.1 to 1.0.1 and uninstall the direct uglify-js dependency.

Member

mgol commented Aug 16, 2017

Another option to fix it is to downgrade grunt-contrib-uglify from 3.0.1 to 1.0.1 and uninstall the direct uglify-js dependency.

@mgol

This comment has been minimized.

Show comment
Hide comment
@mgol

mgol Aug 16, 2017

Member

And example test run on this PR's branch:
screen shot 2017-08-17 at 00 17 23

Android 4.0 no longer times out.

EDIT: I swapped the link for a screenshot to not leave the out-of-tree build on TestSwarm as it may make it harder to figure out regression ranges in the future.

Member

mgol commented Aug 16, 2017

And example test run on this PR's branch:
screen shot 2017-08-17 at 00 17 23

Android 4.0 no longer times out.

EDIT: I swapped the link for a screenshot to not leave the out-of-tree build on TestSwarm as it may make it harder to figure out regression ranges in the future.

@timmywil timmywil removed the Needs review label Nov 27, 2017

@timmywil

This comment has been minimized.

Show comment
Hide comment
@timmywil

timmywil Jan 3, 2018

Member

Closing in favor of #3920, which is pretty much the same except it updates uglify and only sets ie8: true on output options.

Member

timmywil commented Jan 3, 2018

Closing in favor of #3920, which is pretty much the same except it updates uglify and only sets ie8: true on output options.

@timmywil timmywil closed this Jan 3, 2018

@mgol mgol deleted the mgol:android4-test branch Feb 12, 2018

@lock lock bot locked as resolved and limited conversation to collaborators Aug 11, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.