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

VERIFIED FIXED

Status

Firefox OS
Gaia::E-Mail
VERIFIED FIXED
6 years ago
5 years ago

People

(Reporter: asuth, Unassigned)

Tracking

unspecified
x86_64
Linux

Firefox Tracking Flags

(blocking-b2g:tef+, b2g18 verified, b2g18-v1.0.1 verified)

Details

(Whiteboard: [FFOS_perf])

(Reporter)

Description

6 years ago
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.
(Reporter)

Comment 1

6 years ago
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.
I'm happy to help, feel free to ping me in a realtime channel on what you would like to see done.
(Reporter)

Comment 4

5 years ago
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.
Status: NEW → RESOLVED
Last Resolved: 5 years ago
Depends on: 837111
Resolution: --- → FIXED
I know
Whiteboard: [FFOS_perf]
sorry for the last comment, I just wanted to add the whiteboard keyword.
We need this for Bug 841251 that we really need to land now.
blocking-b2g: --- → tef?
tracking-b2g18: --- → ?
(tef+ per comment 7)
blocking-b2g: tef? → tef+
master commit hash : 02056c64bc15f879d5bdb1b4d02aa4b89bc4a5e7
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.
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.
v1-train: a049035ba491a21c5ec7a53bba04b3cf4f5fe53a
status-b2g18: --- → fixed
status-b2g18-v1.0.1: --- → affected
tracking-b2g18: ? → ---
v1.0.1: 7bf22d510b6cae76bd9b41e28c70e0c6684e895e
status-b2g18-v1.0.1: affected → fixed
(Reporter)

Comment 14

5 years ago
(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

5 years ago
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
Status: RESOLVED → VERIFIED
status-b2g18: fixed → verified
status-b2g18-v1.0.1: fixed → verified
You need to log in before you can comment on or make changes to this bug.