960343 - The build system doesn't minify js files from the email app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::E-Mail, defect)

Description

[[:fabrice] Fabrice Desré]

And there are quite a lot of big ones there.

We enter optimize_aggregateJsResources() in webapp-optimize.js but we don't find any js file to aggregate and minify because the only script in index.html is in the body.

Even finding this one would not help a lot I guess since most files are lazy loaded differently.

Comment 1

[[:fabrice] Fabrice Desré]

Asuth told me about turning on uglify2 in apps/email/build/email.build.js 
This helps quite a bit, getting the zip size to 976665 to 583233 bytes
That looks like a no brainer to me, and if we can get a bit more, please do!

Comment 2

[Andrew Sutherland [:asuth] (he/him)]

:fabrice and :jrburke and I discussed this more on #gaia.  Our current plan is to crank up the minification in gaia and in GELAM.  Details:
- We won't change the install-into-gaia flow at this time,  this is okay and won't cause massive patches every time because things will be fairly stable because...
- We will try and keep newlines around as much as possible so that exceptions aren't garbage.
- It's okay to for the optimizer to eat comments.

Comment 3

[James Burke [:jrburke]]

Attachment: GitHub pull request

This pull request turns on uglify2 minification for email, and it contains a bypass Makefile variable, GAIA_EMAIL_MINIFY=none, that can be used in dev to skip uglification, since it can take a bit of time.

The Makefile also limits what it copies out of the /shared directory to avoid uglify2 complaining about some of the files, and to reduce the amount of work.

Here are the application.zip file size comparisons, with different uglify options:

* beautify: keep line returns and indentation
* compress: tries to optimize some code constructions
* mangle: change variables/identifiers to shorter names

Sizes in bytes:

1) 977,680 - no minification
2) 630,064 - beautify no mangle no compress (just remove comments basically)
3) 573,836 - compress, mangle, with beautify on
4) 537,023 - compress, mangle, no beautify (just long lines of JS)

I chose setting 3, so that line breaks are preserved. This gives a good savings on file size while also allowing us to get error messages with a usable line number. However, since uglify2 is modifying the code, if we ever run into trouble with that, where it introduces an error, we may fall back to setting 2, which just removes code comments.

Looking at the application.zip contents, we are pretty aggressive with unnecessary file stripping. There are a couple of things we could still do:

1) trim the .html files from the shared/style and shared/style_unstable. These just seem to be demo pages for the CSS. I did a quick manual removal and this seemed to save about 40-50KB in zip size. As this affects all gaia apps, the bulk savings may be add up, so I will file a separate bug, hopefully with a patch, later today for this.

2) trim some .html files that are inlined in the JS for email. Doing a quick hack, this might reduce the zip size by about 8KB. So I do not think it is worth pursuing this one at this time.

Comment 4

[James Burke [:jrburke]]

Filed bug 960769 to track removal of .html files from building block subcomponents.

Comment 5

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 8361331 [details] [review]
GitHub pull request

The output looks as expected and I noticed no (new) regressions while doing a quick runthrough of the e-mail app's functionality.  And I don't think we've ever tried to do anything clever that would run afoul of the local shortening.

Thanks for the fix!

Comment 6

[James Burke [:jrburke]]

Merged in gaia master:
https://github.com/mozilla-b2g/gaia/commit/e58e11c376048692a2c6830756e135f7c52f48a4

from pull request:
https://github.com/mozilla-b2g/gaia/pull/15430

Comment 7

[James Burke [:jrburke]]

Apparently the changes in Bug 948283 caused an issue with the unstable use in this branch. Reverted above commit and will sort out, then redo the merge.

Comment 8

[James Burke [:jrburke]]

Attachment: GitHub pull request

Same as previous pull request, https://github.com/mozilla-b2g/gaia/pull/15430, but without the shared/style_unstable copy in the email/Makefile since bug 948283 moved those contents into the existing shared/style.

As this is just a small change/merge that is testable by travis succeeding, I  consider the r+ still in effect, and will merge once travis indicates it is happy.

Comment 9

[James Burke [:jrburke]]

Merged on gaia master:
https://github.com/mozilla-b2g/gaia/commit/ae4495912ec6ab2214ee1c95baa25bc49dfd1382

from pull request:
https://github.com/mozilla-b2g/gaia/pull/15569