980391 - Remove navigator.mozMobileConnection uses from the system app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::System, defect)

Description

[Gabriele Svelto [:gsvelto]]

The system app currently uses navigator.mozMobileConnection in multiple places; it's uses should be removed and only navigator.mozMobileConnections should be used instead.

Comment 1

[kumar rishav (:rishav_)]

Hi Gabriele
I like to work on this.Can i get this assigned?
(I saw three bugs like this so i commented on all).
Thanks

Comment 2

[kumar rishav (:rishav_)]

Sorry there are six bugs like this.I like to give a shot to all in this weekend (I am free now,so).So i have asked on every bug.
Thanks

Comment 3

[Lucas]

Hey could I take this one? I'm trying to learn the Mozilla system. Thanks

Comment 4

[Gabriele Svelto [:gsvelto]]

(In reply to Lucas from comment #3)
> Hey could I take this one? I'm trying to learn the Mozilla system. Thanks

Sure, here you go. A little background on this bug since you're getting started. navigator.mozMobileConnection represents a mobile phone connection under Firefox OS, since we currently support multiple SIMs and thus multiple connections this field has been removed and replaced with an array called navigator.mozMobileConnections. The scope of this bug is to remove the last uses of navigator.mozMobileConnection from the system app and to adjust the corresponding unit tests to use the appropriate mock. You can have a look at the fixed bugs under bug 980387 to have an example of how this should be done. Feel free to ping me if you need help.

Comment 5

[Lucas]

(In reply to Gabriele Svelto [:gsvelto] from comment #4)
> (In reply to Lucas from comment #3)
> > Hey could I take this one? I'm trying to learn the Mozilla system. Thanks
> 
> Sure, here you go. A little background on this bug since you're getting
> started. navigator.mozMobileConnection represents a mobile phone connection
> under Firefox OS, since we currently support multiple SIMs and thus multiple
> connections this field has been removed and replaced with an array called
> navigator.mozMobileConnections. The scope of this bug is to remove the last
> uses of navigator.mozMobileConnection from the system app and to adjust the
> corresponding unit tests to use the appropriate mock. You can have a look at
> the fixed bugs under bug 980387 to have an example of how this should be
> done. Feel free to ping me if you need help.

Thanks Gabriele, I'm sorry to ask such a stupid question, but how do I get the source code to work on this? Is it on github? Which package is it?

Comment 6

[kumar rishav (:rishav_)]

(In reply to Lucas from comment #5)
> Thanks Gabriele, I'm sorry to ask such a stupid question, but how do I get
> the source code to work on this? Is it on github? Which package is it?
Hi Lucas
Yeah, source code is on github and hope you are familiar with the working of github.System app of gaia lies here https://github.com/mozilla-b2g/gaia/tree/master/apps/system .The system app currently uses navigator.mozMobileConnection in multiple places; it's uses should be removed and only navigator.mozMobileConnections should be used instead.

Comment 7

[Gabriele Svelto [:gsvelto]]

(In reply to Lucas from comment #5)
> Thanks Gabriele, I'm sorry to ask such a stupid question, but how do I get
> the source code to work on this? Is it on github? Which package is it?

You should start by getting Gaia's source code and setting up your development environment, this is a good place to start:

https://developer.mozilla.org/en-US/Firefox_OS/Platform/Gaia/Hacking

Comment 8

[Lucas]

Thanks, Gabrielle. I am now converting to git after some time with subversion. I struggled a little setting it up, but I should be good now. I'll ask if I have any other issues. Thanks again.

(In reply to Gabriele Svelto [:gsvelto] from comment #7)
> (In reply to Lucas from comment #5)
> > Thanks Gabriele, I'm sorry to ask such a stupid question, but how do I get
> > the source code to work on this? Is it on github? Which package is it?
> 
> You should start by getting Gaia's source code and setting up your
> development environment, this is a good place to start:
> 
> https://developer.mozilla.org/en-US/Firefox_OS/Platform/Gaia/Hacking

Comment 9

[Lucas]

(In reply to Gabriele Svelto [:gsvelto] from comment #7)
> (In reply to Lucas from comment #5)
> > Thanks Gabriele, I'm sorry to ask such a stupid question, but how do I get
> > the source code to work on this? Is it on github? Which package is it?
> 
> You should start by getting Gaia's source code and setting up your
> development environment, this is a good place to start:
> 
> https://developer.mozilla.org/en-US/Firefox_OS/Platform/Gaia/Hacking


Hi Gabriele, 
Can you tell me if the variable realMozMobileConnection in apps/communications/contacts/test/unit/views/details_test.js obsolete? It is also found in many of the other files that still contain the variable navigator.mozMobileConnection. What about other singular noun variables for connections? Are these two be removed as well? I've noticed that there are many involved with navigator.mozMobileConnection. 
Thanks again.
-Lucas

Comment 10

[kumar rishav (:rishav_)]

Hi Lucas
Yes,it's obsolete, you have to change it to realMozMobileConnections.You have to replace all navigator.mozMobileConnection with navigator.mozMobileConnections used in system app.Have a look on this bug, this is some what similar :( https://bugzilla.mozilla.org/show_bug.cgi?id=980395 &  https://bugzilla.mozilla.org/show_bug.cgi?id=980402 ).
Hope this will help
Thanks

Comment 11

[Gabriele Svelto [:gsvelto]]

(In reply to Lucas from comment #9)
> Can you tell me if the variable realMozMobileConnection in
> apps/communications/contacts/test/unit/views/details_test.js obsolete?

Yes it is, however this bug covers only replacing it in the system app which is under apps/system

> What about other singular noun variables for
> connections? Are these two be removed as well?

There's two variables involved here navigator.mozMobileConnection (singular) and navigator.mozMobileConnections (plural). The singular form is obsolete and should be replaced by the plural one. One thing to note is that in the obsolete form navigator.mozMobileConnection represented a single connection while navigator.mozMobileConnections is an array so depending on the context you can either pick the first element of this array or a specific one.

Comment 12

[Lucas]

(In reply to Gabriele Svelto [:gsvelto] from comment #11)

Ok, great. Is there any tests I can run before sending a pull request?

Comment 13

[Lucas]

(In reply to Gabriele Svelto [:gsvelto] from comment #11)

Before that, can you help me understand what you mean by "to adjust the corresponding unit tests to use the appropriate mock."? I'm not sure where I would find this. Thanks

Comment 14

[Gabriele Svelto [:gsvelto]]

(In reply to Lucas from comment #12)
> Ok, great. Is there any tests I can run before sending a pull request?

You should run the unit-tests as described here:

https://developer.mozilla.org/en-US/Firefox_OS/Platform/Automated_testing/Gaia_unit_tests

Make sure all tests from the system app run correctly. Once you send a pull request you'll automatically trigger also an automated test run on Travis. You'll see it in your PR or you'll be able to find it here:

https://github.com/mozilla-b2g/gaia

The patch won't be considered finished unless that test run passes completely.

(In reply to Lucas from comment #13)
> Before that, can you help me understand what you mean by "to adjust the
> corresponding unit tests to use the appropriate mock."? I'm not sure where I
> would find this.

mozMobileConnection is an object that corresponds to a cellular mobile phone connection so it's obviously not available on your local machine when running tests. In order to work around this issue we use mocks in the test. A mock is a fake JS object that reproduces the behavior of the original object.

The tests that use navigator.mozMobileConnection make use of the following mock:

https://github.com/mozilla-b2g/gaia/blob/master/shared/test/unit/mocks/mock_navigator_moz_mobile_connection.js

This should be replaced with this other mock that's been made to simulator navigator.mozMobileConnections instead:

https://github.com/mozilla-b2g/gaia/blob/master/shared/test/unit/mocks/mock_navigator_moz_mobile_connections.js

More information on writing unit-tests is available here:

https://developer.mozilla.org/en-US/Firefox_OS/Platform/Automated_testing/Writing_Gaia_Unit_Tests

Comment 15

[Gabriele Svelto [:gsvelto]]

Attachment: [PULL REQUEST] Remove the last remaining use of navigator.mozMobileConnection

Most of the other uses of navigator.mozMobileConnection have already been removed in other bugs. Let's clean this last use and close this.

Comment 16

[(inactive after 6/18) Alive Kuo [:alive]]

Comment on attachment 8554245 [details] [review]
[PULL REQUEST] Remove the last remaining use of navigator.mozMobileConnection

Simple enough!

Comment 17

[Gabriele Svelto [:gsvelto]]

Green try run: https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=150470b71a26

Pushed to gaia/master 9d9278b38a71180b49ad59f80440ad22b813d1c8

https://github.com/mozilla-b2g/gaia/commit/9d9278b38a71180b49ad59f80440ad22b813d1c8