980395 - Remove navigator.mozMobileConnection uses from the FTU app

Status: RESOLVED FIXED

(Firefox OS Graveyard :: Gaia::First Time Experience, defect)

Description

[Gabriele Svelto [:gsvelto]]

The FTU app still uses navigator.mozMobileConnection in one place and in unit-tests; those uses should be removed and made to rely only on navigator.mozMobileConnections.

Comment 1

[kumar rishav (:rishav_)]

Hi Gabriele
I like to work on this.Can i get this assigned?
Thanks

Comment 2

[Gabriele Svelto [:gsvelto]]

(In reply to kumar rishav from comment #1)
> Hi Gabriele
> I like to work on this.Can i get this assigned?

Sure, here you go.

Comment 3

[kumar rishav (:rishav_)]

Attachment: Bug 980395 - Remove navigator.mozMobileConnection uses from the FTU app

Hi Gabriele
I have made required changes.Have a look on this PR ?
Thanks

Comment 4

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8406300 [details] [review]
Bug 980395 - Remove navigator.mozMobileConnection uses from the FTU app

(In reply to kumar rishav from comment #3)
> I have made required changes.Have a look on this PR ?

I can't review your PR as I'm not a peer so I'll give you a feedback. Once this is ready you'll have to ask for a FTU peer to review your patch.

I've left some comments on the PR but currently it is incomplete, with your changes the unit tests don't run at all. You have to make sure the unit-tests run and do so correctly, ask for feedback again once you have them running properly. I suggest you to run the tests w/o your changes and understand what they're doing before doing further changes.

Also you should remove the apps/communications/ftu/test/unit/mock_navigator_moz_mobile_connection.js file as part of the PR as it won't be needed anymore once all the changes have been made.

Comment 5

[kumar rishav (:rishav_)]

Attachment: Bug 980395 - Remove navigator.mozMobileConnection uses from the FTU app

Hi Gabriele

I have made the suggested changes and unit test is running successfully.
Have a look and give your feedback.Also i have asked fernando to review the PR.

Once again Thanks a lot Gabriele :)

Comment 6

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8406397 [details] [review]
Bug 980395 - Remove navigator.mozMobileConnection uses from the FTU app

Looks good to me, thanks for taking this.

Comment 7

[Fernando Campo (:fcampo)]

Comment on attachment 8406397 [details] [review]
Bug 980395 - Remove navigator.mozMobileConnection uses from the FTU app

Quick and easy for me, all the review was done already :p (thanks Gabriele).

And thanks kumar for taking this, code looks good.

Just a small detail for readability (comment on github).

So, r+ proven the changes (just style, shouldn't give any problems), and merge once travis gets green again.

Comment 8

[kumar rishav (:rishav_)]

Attachment: Bug 980395 - Remove navigator.mozMobileConnection uses from the FTU app

Hi  Fernando Campo
I have done the changes as per your suggestion.Have a look on final PR?
Thanks

Hi gabriele
After getting this patch merged,can you assign me other bugs i have commented before?
Thanks

Comment 9

[Gabriele Svelto [:gsvelto]]

Thanks to both, Travis was green so I merged the PR to gaia/master 72c42e5568397425c8b8a96ad0aa8f6778d026f5

https://github.com/mozilla-b2g/gaia/commit/72c42e5568397425c8b8a96ad0aa8f6778d026f5