908117 - [B2G][dialer][tongxiao]Could not send out the USSD command when the USSD number is saved in the contacts

Status: RESOLVED FIXED

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

Description

[lecky]

User Agent: Mozilla/4.0 (compatible; MSIE 8.0; Windows NT 6.1; Trident/4.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; MALC; aff-kingsoft-ciba; .NET4.0C; .NET4.0E; Tablet PC 2.0)

Steps to reproduce:

1.Save the USSD number like *100# , into the contacts.
2.Call the contact which save the USSD number.
3.Check whether  the USSD command  is sended out.


Actual results:

the phone make a ordinary call which phone number is the USSD number.


Expected results:

the phone  send out the USSD command .

Comment 1

[Fernando Jiménez Moreno [:ferjm]]

This is not device specific

Comment 2

[lecky]

Will you solve it?

Comment 3

[Fernando Jiménez Moreno [:ferjm]]

Attachment: wip

I still need to do further testing and fix the unit tests, but I'd like to have some feedback in the meantime.

Comment 4

[Fernando Jiménez Moreno [:ferjm]]

Attachment: wip

Now the good one

Comment 5

[Etienne Segonzac (:etienne)]

Comment on attachment 799425 [details] [diff] [review]
wip

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

Sounds good!
A few small comments below.

BTW, :Rik will be a great reviewer for the final patch :)

::: apps/communications/dialer/js/mmi.js
@@ +17,5 @@
>    _pendingRequest: null,
>  
> +  init: function mm_init(callback) {
> +    if (this._ready) {
> +      callback();

we should check that |callback| is not undefined.

::: apps/communications/dialer/test/unit/mmi_test.js
@@ +14,5 @@
> +}
> +
> +if (!this.MobileOperator) {
> +  this.MobileOperator = null;
> +}

you should use the mocksHelper and you won't need this :)

@@ +36,5 @@
> +    realMobileOperator = window.MobileOperator;
> +    window.MobileOperator = MockMobileOperator;
> +
> +    realMobileConnection = window.navigator.mozMobileConnection;
> +    window.navigator.mozMobileConnection = MockMozMobileConnection;

this neither

@@ +48,5 @@
>  
>    teardown(function() {
> +    window.LazyLoader = realLazyLoader;
> +    window.MobileOperator = realMobileOperator;
> +    window.navigator.mozMobileConnection = realMobileConnection;

this neither

Comment 6

[lecky]

 Any update?

Comment 7

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v1

Comment 8

[Beatriz Rodríguez [:brg]]

Changing the flag to koi? because this is a generic failure in 1.2( I mean not device specific) and a possible certification blocker for the close future.
Thanks Fernando for fixing it!

Comment 9

[Fernando Jiménez Moreno [:ferjm]]

I'll rebase this patch as soon as bug 915038 lands.

Comment 10

[Anthony Ricaud (:rik)]

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

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

The dialer changes look identical to bug 915038. Except for the extra mock_mobile_operator.js. A similar file already exists in apps/system, you should probably use that one.

For the contacts changes, I'm not a peer so you should ask someone else to review it. But since I always have to give my opinion, I'm still giving some comments :)

::: apps/communications/contacts/js/contacts.js
@@ +358,4 @@
>    var callOrPick = function callOrPick(number) {
>      if (ActivityHandler.currentlyHandling) {
>        ActivityHandler.postPickSuccess({ number: number });
> +    } else if (MmiManager.isMMI(number)) {

Could we lazy load MmiManager only now instead of for every launch?

@@ +666,5 @@
>        '/facebook/style/curtain_frame.css',
>        '/contacts/style/status.css',
> +      '/contacts/style/fb_extensions.css',
> +      '/dialer/js/mmi.js',
> +      '/shared/js/lazy_l10n.js'

Why are you loading lazy_l10n.js here?

Comment 11

[Fernando Jiménez Moreno [:ferjm]]

Attachment: v2

Now that bug 915038 landed, the patch is far more simpler :)

Comment 13

[Jose Manuel Cantera]

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

thanks. now is working ok.

PS: It is better, specially for reviewers to create a GH PR rather than a diff.

Comment 14

[Fernando Jiménez Moreno [:ferjm]]

Thanks!

https://github.com/mozilla-b2g/gaia/commit/78a3e2509b979bd226f01f557bdf229fd3faa42e

Comment 15

[Anthony Ricaud (:rik)]

I should have mentioned it earlier but no new tests for this makes me sad :( (http://4.bp.blogspot.com/-Sp4xX4sCmEI/UGCThDfZRpI/AAAAAAAAAkg/CewxrnBjr10/s1600/sad-panda-podiatrist_medium.jpg)

Comment 16

[lecky]

Hi,
  Will this problem be fixed in the v1.1hd?
thank you!

Comment 17

[Wayne Chang [:wchang]]

no, it will be considered for v1.2 only.

(In reply to lecky from comment #16)
> Hi,
>   Will this problem be fixed in the v1.1hd?
> thank you!

Comment 18

[Fernando Jiménez Moreno [:ferjm]]

(In reply to Anthony Ricaud (:rik) from comment #15)
> I should have mentioned it earlier but no new tests for this makes me sad :(
> (http://4.bp.blogspot.com/-Sp4xX4sCmEI/UGCThDfZRpI/AAAAAAAAAkg/CewxrnBjr10/
> s1600/sad-panda-podiatrist_medium.jpg)

Don't be sad mate. Bug 919447 :)

Comment 19

[Joe Cheng [:jcheng] (please needinfo)]

triage: koi+

Comment 20

[James Lal [:lightsofapollo]]

Uplifted 78a3e2509b979bd226f01f557bdf229fd3faa42e to:
v1.2: 2acd0dbe4d3d0870e960ab5e087ce520dc97279f

Comment 22

[lecky]

Hi Beatriz:
   As Comment 17, they will not be considered on v1.1 now.
   Do you accept that we do not modify this problem?

Comment 23

[Beatriz Rodríguez [:brg]]

(In reply to lecky from comment #22)
> Hi Beatriz:
>    As Comment 17, they will not be considered on v1.1 now.
>    Do you accept that we do not modify this problem?
Agree with comment 17. Patch is available for v1.2!