814273 - [email] Replace concatenated gaia-email-opt.js with the files that make it up (as a vendor code drop)

Status: VERIFIED FIXED

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

Description

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

Andreas has observed that gaia-email-opt.js is not particularly readable since it's a single giant concatenated file.  While I think there was some desire to dissolve gaia-email-libs-and-more into the gaia repo, I believe Andreas and Vivien have agreed that we can keep gaia-email-libs-and-more and just copy across our files so it's a flat code drop.  I think every repo wins because it saves gaia from all of the build steps of our node library dependencies, etc.

The work effort here mainly entails:
- Replace the install-gaia-email-opt and gaia-email-opt.js targets with something that rsyncs all of the source files across to the gaia dir.  Since the source files are spread aboot a bit, this may be a few rsyncs.
- Update index.html to be chock-full'o'script tags for all of our many files.

It's conceivable that jrburke's volo or r.js tools may be able to actually automate some/all of this, especially if we contribute a patch.  Specifically, since the r.js optimizer already knows how to build gaia-email-opt.js given a path map and by performing scanning for define()/require() calls, it might be able to do the rsync-esque thing (we will need deletion too, and it would be nice not to have every step be rm -rf and then install everything) and update the script tags.  Or not, it's pretty specific and not the standard idiom.

Comment 1

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

Er, and there will very much be known badness with this approach where people will get confused and submit patches against gaia rather than gaia-email-libs-and-more.  I think we can mitigate these problems by:

- Having a very short and explicit README.md in each folder that we synchronize across.
- Putting a comment about the code being from gaia-email-libs-and-more as part of our boilerplate at the top of the gelam code.
- Ideally putting an automated comment at the top of each auto-converted node library that namechecks the library it's actually from and maybe also mentioning gaia-email-libs-and-more.

Comment 2

[James Burke [:jrburke]]

I'm happy to help, feel free to ping me in a realtime channel on what you would like to see done.

Comment 3

[James Burke [:jrburke]]

Pull request started at: https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/86

Comment 4

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

landed on gaia-email-libs-and-more/master:
https://github.com/mozilla-b2g/gaia-email-libs-and-more/pull/124

landed on gaia/master:
https://github.com/mozilla-b2g/gaia/pull/8070

Many thanks, James!


If the minification bug (bug 837111) gets uplifted, we will want to request uplift to v1-train too.

Comment 5

[Julien Wajsberg [:julienw]]

I know

Comment 6

[Julien Wajsberg [:julienw]]

sorry for the last comment, I just wanted to add the whiteboard keyword.

Comment 7

[Julien Wajsberg [:julienw]]

We need this for Bug 841251 that we really need to land now.

Comment 8

[Michael Vines [:m1] [:evilmachines]]

(tef+ per comment 7)

Comment 9

[Julien Wajsberg [:julienw]]

master commit hash : 02056c64bc15f879d5bdb1b4d02aa4b89bc4a5e7

Comment 10

[Julien Wajsberg [:julienw]]

Uplifting this will take the part of Bug 814257 that landed (hash 0f87a505a1b1972dff92de35a04ec618492f31c9) too. There is no way we can uplift this without that.

Testing to be sure everything is ok.

Comment 11

[Julien Wajsberg [:julienw]]

So, there is a regression:

* add an account
* remove the account
* kill the app
* launch the app

=> black screen with errors in logcat

I spent 1 hour trying to find why this is happening and couldn't find it (especially I don't see a difference in these parts of the code) so I'll uplift as is (because we need to have Bug 841251) and we could tackle this problem separatly. I think it happens on master too.

Comment 12

[Julien Wajsberg [:julienw]]

v1-train: a049035ba491a21c5ec7a53bba04b3cf4f5fe53a

Comment 13

[Julien Wajsberg [:julienw]]

v1.0.1: 7bf22d510b6cae76bd9b41e28c70e0c6684e895e

Comment 14

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

(In reply to Julien Wajsberg [:julienw] from comment #11)
> So, there is a regression:
> 
> * add an account
> * remove the account
> * kill the app
> * launch the app
> 
> => black screen with errors in logcat

I filed bug 850904 on this; I believe I correctly analyzed the root cause.  As you posit, this isn't actually a regression but rather a long-standing bug affecting all branches.

Comment 15

[mdaniloff]

Verified fixed on Unagi V.1.0.1 and V.1.1

No black screen with errors appear if the user relaunches the email app from a recently removed email account


Unagi V.1.0.1
Unagi Build ID: 20130328070202
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/c516d7e67150
Gaia: d40dcdd112f12e2a5a0d1de46451670918fd4369

V.1.1
Unagi Build ID: 20130403070204
Kernel Date: Dec 5
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/d467369d1b0c
Gaia: 06e0e5ce42bdfb62bdbe38271de6b5b2d9e40e75