884349 - (mmi-result-imei) [MMI] Use MMIResult for get IMEI

Status: RESOLVED FIXED

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

Comment 1

[Fernando Jiménez Moreno [:ferjm]]

leo? as part of the supplementary services translations

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

This patch includes the consumption of the MMIResult sent by the platform as DOMRequest.result for a get IMEI request via MMI.

As you can see, we include now the type of service associated to the MMI request, which lets us specify a more specific header for the MMI screen.

There are a few TODO comments that needs to be fixed in other bugs blocked by Bug 883178. All these patches should land together so we don't break any MMI functionality.

Comment 4

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 5

[Etienne Segonzac (:etienne)]

Comment on attachment 765230 [details] [diff] [review]
v1

Review of attachment 765230 [details] [diff] [review]:
-----------------------------------------------------------------

Rik, feel like reviewing this?

Comment 6

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 7

[Anthony Ricaud (:rik)]

Comment on attachment 765386 [details] [diff] [review]
v1

Review of attachment 765386 [details] [diff] [review]:
-----------------------------------------------------------------

If you receive a voicechange event after displaying the IMEI, the title will switch to _('ussd-services') instead of keeping _('scImei'). Maybe updateHeader should be a no-op if we've already called showMessage with a not-undefined header parameter?

Also, I'm sad to see some disabled functionality and tests but given that the other bugs will be leo+, I'll look the other way :)

::: apps/communications/dialer/js/mmi.js
@@ +141,5 @@
> +      case 'scImei':
> +        if (mmiResult.statusMessage) {
> +          message = {
> +            type: 'mmi-success',
> +            title: this._('scImei'),

You're missing this entry in dialer.en-US.properties.

@@ +167,5 @@
> +
> +        // TODO: Bug 884343. Use MMIResult for Call Forwarding related
> +        //       functionality.
> +        break;
> +      default:

If we go to this case, we're receiving an unexpected serviceCode, shoud we do console.error or something ?

Comment 8

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v2

Comment 9

[Fernando Jiménez Moreno [:ferjm]]

Thanks Anthony!

(In reply to Anthony Ricaud (:rik) from comment #7)
> Comment on attachment 765386 [details] [diff] [review]
> v1
> 
> Review of attachment 765386 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> If you receive a voicechange event after displaying the IMEI, the title will
> switch to _('ussd-services') instead of keeping _('scImei'). Maybe
> updateHeader should be a no-op if we've already called showMessage with a
> not-undefined header parameter?
> 

Actually, I've removed updatedHeader. We set the operator name as header if we get an USSD message, so I can't think about any reason for setting it again.

> Also, I'm sad to see some disabled functionality and tests but given that
> the other bugs will be leo+, I'll look the other way :)

:)

> 
> ::: apps/communications/dialer/js/mmi.js
> @@ +141,5 @@
> > +      case 'scImei':
> > +        if (mmiResult.statusMessage) {
> > +          message = {
> > +            type: 'mmi-success',
> > +            title: this._('scImei'),
> 
> You're missing this entry in dialer.en-US.properties.
> 

The strings are included in Bug 879032

> @@ +167,5 @@
> > +
> > +        // TODO: Bug 884343. Use MMIResult for Call Forwarding related
> > +        //       functionality.
> > +        break;
> > +      default:
> 
> If we go to this case, we're receiving an unexpected serviceCode, shoud we
> do console.error or something ?

We should never get there... but I added a console.log

Comment 11

[Wayne Chang [:wchang]]

Attachment: IMEI displayed with *#06*

Comment 12

[Noemí Freire (:noemi)]

(In reply to Wayne Chang [:wchang] from comment #11)
> Created attachment 765847 [details]
> IMEI displayed with *#06*

IMEI info is displayed but the title is showing "operator name" instead of "IMEI" which should be the expected title. Moreover the patch attached in this bug put the gaia basis for all the localization work so it needs to be taken. Because of this, we really encourage to take this.

Comment 13

[Fernando Jiménez Moreno [:ferjm]]

Comment on attachment 765441 [details] [diff] [review]
v2

Review of attachment 765441 [details] [diff] [review]:
-----------------------------------------------------------------

I'm clearing the r? flag until we are done with bug 885701 and bug 883178

Comment 14

[bhavana bajaj [:bajaj]]

Blocking only because this may be needed for other blockers 874000,879680

Comment 15

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v2

Comment 16

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v2

Now the correct one...

Comment 17

[Anthony Ricaud (:rik)]

Comment on attachment 767667 [details] [diff] [review]
v2

Review of attachment 767667 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with this one small change. Thanks!

::: apps/communications/dialer/js/mmi.js
@@ -179,4 @@
>  
>      var message;
>      switch (evt.type) {
> -      case 'voicechange':

If we don't listen to this anymore, you should remove the addEventListener in init().

Comment 18

[Anthony Ricaud (:rik)]

Comment on attachment 767667 [details] [diff] [review]
v2

Review of attachment 767667 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with this one small change. Thanks!

::: apps/communications/dialer/js/mmi.js
@@ -179,4 @@
>  
>      var message;
>      switch (evt.type) {
> -      case 'voicechange':

If we don't listen to this anymore, you should remove the addEventListener in init().

Comment 19

[Fernando Jiménez Moreno [:ferjm]]

Right! I left it there, so you had something to comment (just kidding :P).

Thanks for the quick review Anthony!

Comment 20

[Alex Keybl [:akeybl]]

Please land asap since this requires late-l10n and the end of Junes is our cutoff on new strings.

Comment 21

[Fernando Jiménez Moreno [:ferjm]]

This can't be land until bug 883178 lands. I'm doing my best to get that one done, but it's not being easy as it is quite different in m-c and b2g18 :(. I'm hoping to get it done today.

Comment 22

[Fernando Jiménez Moreno [:ferjm]]

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

Comment 23

[Gareth Aye [:gaye] (back from PTO)]

Uplifted b4314d78e75355cdd2c1a9250f97344f402717d3 to:
v1-train: 9024630641cde05f85edfac261a6c3638dbef4ea

Comment 24

[Gareth Aye [:gaye] (back from PTO)]

v1.1.0hd: 9024630641cde05f85edfac261a6c3638dbef4ea

Comment 25

[Mila Davydova]

When user dials *#06# MMI code the IMEI number is returned, and it has "IMEI" as title. 
 The "IMEI" title is displayed as "IMEI" in all of the checked languages and the "Send" button located in the header appears translated. Issue was checked on following languages: Polish, Czech, Dutch, Russian, Greek, Slovak, Romanian, Brazilian-Portuguese, Hungarian, German, Spanish, Turkish, Croatian, Catalan, Serbian- Cyrillic and Serbian - Latin.

Leo COM RIL Build ID: 20130814041202
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18/rev/15813d776a69
Gaia: dd3959fa74e356a528daa76ffee14c2c728a4b56
Platform Version: 18.1
RIL Version: 01.01.00.019.190 
Firmware version: D300f080