1119544 - Remove the unnecessary l10n code from FMD

Status: RESOLVED FIXED

(Firefox OS Graveyard :: FindMyDevice, defect)

Description

[Zibi Braniecki [:zbraniecki][:gandalf]]

It seems that findmydevice/index.html doesn't need l10n.js. That would save us from having to analyze it at build time.

Comment 1

[Zibi Braniecki [:zbraniecki][:gandalf]]

Here's a branch with the code: https://github.com/zbraniecki/gaia/tree/1119544-remove-l10njs-from-fmd

Unfortunately it breaks test/units/findmydevice_test.js in the way that I cannot fix because I cannot get those tests to run locally even without any changes :(

I could use your help guys!

Comment 2

[:gerard-majax]

If you don't give us anything linked to a failure, I don't see how we can help. How urgent is this ? Can this wait a couple of days (still PTO for now) ?

Comment 3

[Zibi Braniecki [:zbraniecki][:gandalf]]

(In reply to Alexandre LISSY :gerard-majax from comment #2)
> How urgent is this ? Can this wait a couple of days (still PTO for now) ?

Not urgent. :) Feel free to ignore it until you're back :)

I'll provide the failure data when I rerun it.

Comment 4

[:gerard-majax]

I've opened a PR https://github.com/mozilla-b2g/gaia/pull/27378 to check for gaia-try errors, and I'm triggering tests locally to see.

Comment 5

[:gerard-majax]

Triggering findmydevice_test.js, I see "line 1000: ReferenceError: Config is not defined"

Comment 6

[:gerard-majax]

Ok, we depend on config.js

Comment 7

[:gerard-majax]

Damn that's trivial: we used to avoid initializing the FMD object, with the part of code you removed:

> @@ -33,14 +31,6 @@ suite('FindMyDevice >', function() {
>    mocksForFindMyDevice.attachTestHelpers();
>  
>    suiteSetup(function(done) {
> -    realL10n = navigator.mozL10n;
> -    navigator.mozL10n = window.MockL10n;
> -    sinon.stub(navigator.mozL10n, 'once', function(callback) {
> -      // we don't need to actually initialize FMD
> -      // for these unit tests, and it saves us from
> -      // mocking many objects
> -    });
> -
>      realMozId = navigator.mozId;
>      // attempting to stub only the request method of mozId,
>      // as in |sinon.stub(navigator.mozId, 'request', ...)|,

Comment 8

[:gerard-majax]

I've fixed the FMD code that made the bad breakage, now we just have two tests failing:

> ✓ [findmydevice-test/unit/findmydevice_test.js] ensure retry counter is reset on enable
> ✓ [findmydevice-test/unit/findmydevice_test.js] retryCount is not incremented on error if registered
> ✓ [findmydevice-test/unit/findmydevice_test.js] retryCount is incremented on error when not registered
> ✓ [findmydevice-test/unit/findmydevice_test.js] fields from coordinates are included in server response
> ✓ [findmydevice-test/unit/findmydevice_test.js] error message is included in the server response
> 1) [findmydevice-test/unit/findmydevice_test.js] request client id when invalidated
> ✓ [findmydevice-test/unit/findmydevice_test.js] refresh authentication when attempting to disable
> ✓ [findmydevice-test/unit/findmydevice_test.js] setting an alarm releases a wakelock
> ✓ [findmydevice-test/unit/findmydevice_test.js] contact the server on alarm
> ✓ [findmydevice-test/unit/findmydevice_test.js] report to the server when disabled
> ✓ [findmydevice-test/unit/findmydevice_test.js] track and ring are cancelled on LOCKSCREEN_CLOSED, passcode set
> ✓ [findmydevice-test/unit/findmydevice_test.js] track and ring continue on LOCKSCREEN_CLOSED, no passcode set
> 2) [findmydevice-test/unit/findmydevice_test.js] wakelocks are released on unregistered clientID change
> ✓ [findmydevice-test/unit/findmydevice_test.js] wakelocks are released when registering while already registering
>   [findmydevice-test/unit/findmydevice_test.js] findmydevice.current-clientid behavior
> ✓ [findmydevice-test/unit/findmydevice_test.js] invalidate client id when logged in
> ✓ [findmydevice-test/unit/findmydevice_test.js] invalidate client id when logged out
>   [findmydevice-test/unit/findmydevice_test.js] findmydevice.can-disable behavior
> ✓ [findmydevice-test/unit/findmydevice_test.js] set findmydevice.can-disable to false when logged out
> ✓ [findmydevice-test/unit/findmydevice_test.js] don't set findmydevice.can-disable on logout if not registered
> ✓ [findmydevice-test/unit/findmydevice_test.js] allow disabling when clientid matches the state
> ✓ [findmydevice-test/unit/findmydevice_test.js] disallow disabling when clientid doesn't match the state
>   [findmydevice-test/unit/findmydevice_test.js] set alarm on server interaction
> ✓ [findmydevice-test/unit/findmydevice_test.js] alarm is set on successful server response
> ✓ [findmydevice-test/unit/findmydevice_test.js] alarm is set on server response even when disabled

Comment 9

[:gerard-majax]

Repushed on my branch, and it's locally green.
Try: https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=8d7c952169e5

Comment 10

[Zibi Braniecki [:zbraniecki][:gandalf]]

(In reply to Alexandre LISSY :gerard-majax from comment #7)
> Damn that's trivial: we used to avoid initializing the FMD object, with the
> part of code you removed:

Yeah! I was trying to get around that with stupid flags... Your solution looks so clean :)

Thanks Alexandre!

Comment 11

[Zibi Braniecki [:zbraniecki][:gandalf]]

What's the next step here?

Comment 12

[:gerard-majax]

(In reply to Zibi Braniecki [:gandalf] from comment #11)
> What's the next step here?

I don't know, you picking up my fixes and landing this ? Please make it fast, I need to change code right here for bug 1103560.

Comment 13

[:gerard-majax]

Forget what I said, my changes will be on apps/system/js/findmydevice_launcher.js

Comment 14

[Zibi Braniecki [:zbraniecki][:gandalf]]

Attachment: pull request

Comment 15

[:gerard-majax]

Comment on attachment 8555377 [details] [review]
pull request

I should not be reviewing this, since I did the most dangerous fixes there :)

Comment 16

[Guilherme Gonçalves [:ggp]]

Comment on attachment 8555377 [details] [review]
pull request

Thank you!

Comment 17

[Zibi Braniecki [:zbraniecki][:gandalf]]

Commit: https://github.com/mozilla-b2g/gaia/commit/41e6331d6acf0f621bfe1db7bee675f693a60709
Merge: https://github.com/mozilla-b2g/gaia/commit/9d2378a9ef092ab1fc15c3a9f7fc4171aab59d57

Comment 18

[Zibi Braniecki [:zbraniecki][:gandalf]]

Thanks for help Alexandre!