1070825 - [Dialer] Connect multiple way calls with same number but difference extension number.

Status: VERIFIED FIXED

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

Description

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

In some special scenario we need to connect multiple way calls with same number but difference extension  number.

For example,
1. Call #1234
2. connected
3. enter ext. number #001
4. connected
5. Call #1234 again.

We found Dialer APP will avoid user dial same number again, we're just curious that do we need to care about this issue in APP side or does it make sense just let carrier handle it?

https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/telephony_helper.js#L77

Comment 1

[GaryChen [:GaryChen][:PYChen][:gchen][:陳柏宇]]

Hi dwi2,
   Can you help on this issue?
   Thanks.

Comment 3

[Tzu-Lin Huang [:dwi2][:tzhuang]]

taking this

Comment 4

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Hi Etienne, Rik,

We are proposing to remove the check of dialing the same number twice in telephony_helper.js
(remove lines from https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/telephony_helper.js#L78 to L85)
Because rejecting call or not should be a decision of carrier, not ME. 

I am wondering if you have any concern of this change?

I've also test a scenario of dialing the same number twice on flame with and without the check at https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/telephony_helper.js#L78

My STR is like:
1. On device A, make a MO call to another device, say B
2. Answer the call on device B.
3. On device A, tap home and enter dialer again
4. call the number of B again

With the check (which is current implementation on master), the result is:
5. the phone call made at step 1 is on hold, and device B stays on dialer app, no specific message is shown.

Without the check (proposed modification)
5. the phone call made at step 1 is on hold, and device B is ringing and waiting for the second call to connect. (This behavior actually relies on how carriers handle it, it could be another case in different carrier)

Comment 5

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Hi Etienne and Rik,

On second thoughts, remove the check at from https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/telephony_helper.js#L78
could introduce regression in bug 917222. 

A proper way in my mind is to make sure we're not dialing the same number twice only if the existing call is in 'dialing',  'alerting', or 'connecting' state. For other cases, we simply let carrier make the decision to reject call or not. 
How do you think?

Comment 6

[Etienne Segonzac (:etienne)]

(In reply to Tzu-Lin Huang [:dwi2][:tzhuang] from comment #5)
> Hi Etienne and Rik,
> 
> On second thoughts, remove the check at from
> https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/
> js/telephony_helper.js#L78
> could introduce regression in bug 917222. 

Exactly.

> 
> A proper way in my mind is to make sure we're not dialing the same number
> twice only if the existing call is in 'dialing',  'alerting', or
> 'connecting' state. For other cases, we simply let carrier make the decision
> to reject call or not. 
> How do you think?

Ideally we should look at what Hsin-Yi suggested here https://bugzilla.mozilla.org/show_bug.cgi?id=917222#c19 but what you're suggesting could work too (as long as we triple-check that we're not regression bug 917222)

Comment 7

[Anthony Ricaud (:rik)]

I think we can do a bit better than at the time of bug 917222. When we use dial() now, we're waiting for a promise. So Gecko could check if we can make that call and reject the promise, without introducing a new method on Telephony API.

Comment 8

[Tzu-Lin Huang [:dwi2][:tzhuang]]

(In reply to Anthony Ricaud (:rik) from comment #7)
> I think we can do a bit better than at the time of bug 917222. When we use
> dial() now, we're waiting for a promise. So Gecko could check if we can make
> that call and reject the promise, without introducing a new method on
> Telephony API.

Since dial() return promise now, does it mean that we could check if the promise resolved/rejected or not instead of checking dialing the same number twice at https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/telephony_helper.js#L78 ?

That is to say, in TelephonyHelper#startDial() L78 to L85, we change the logic to: if callPromise is not yet resolved/reject, we immediately return.

Aknow, could you help to confirm my proposal here? Thanks

Comment 9

[Szu-Yu Chen [:aknow] (inactive)]

(In reply to Tzu-Lin Huang [:dwi2][:tzhuang] from comment #8)
> (In reply to Anthony Ricaud (:rik) from comment #7)
> > I think we can do a bit better than at the time of bug 917222. When we use
> > dial() now, we're waiting for a promise. So Gecko could check if we can make
> > that call and reject the promise, without introducing a new method on
> > Telephony API.
> 
> Since dial() return promise now, does it mean that we could check if the
> promise resolved/rejected or not instead of checking dialing the same number
> twice at
> https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/
> js/telephony_helper.js#L78 ?
> 
> That is to say, in TelephonyHelper#startDial() L78 to L85, we change the
> logic to: if callPromise is not yet resolved/reject, we immediately return.
> 
> Aknow, could you help to confirm my proposal here? Thanks

Actually, I don't think that there will be any regression if you simply remove the checking for dialing the same number twice. For current promise architecture, it should work well.

Could you try it?

If you do the 2nd dialing too fast, you might get a promise rejection (INVALID_STATE). That means gecko is still processing the 1st request and cannot serve the 2nd one at the moment.

Comment 10

[Tzu-Lin Huang [:dwi2][:tzhuang]]

I tested that calling navigator.mozTelephony.dial() twice in a row, the second promise did get rejected with the cause: 'InvalidStateError'.

So I think it is ok to remove the check for dialing the same number twice without regression of bug 917222.

Comment 11

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Attachment: Pull request

Come up a patch as per comment 9

Hi Rik,
Could you help to review the patch? thanks

Comment 12

[Anthony Ricaud (:rik)]

Comment on attachment 8494925 [details] [review]
Pull request

Pretty cool that we can remove code and fix bugs ;)

Comment 13

[Tzu-Lin Huang [:dwi2][:tzhuang]]

landed on master

https://github.com/mozilla-b2g/gaia/commit/3130c1e39dc9cf80af59ae10335d3bfb941e8479

Comment 14

[Tzu-Lin Huang [:dwi2][:tzhuang]]

[Blocking Requested - why for this release]:
This affects 2.0 and later version. Raise 2.0?

According to comment 4, without this patch:
the phone call made at step 1 is on hold, and device B stays on dialer app, no specific message is shown.

Comment 15

[Rachelle Yang [:ryang][[e-mail]]]

Hi Tzu-Lin,May you please kindly let us know what's the user impact.
Is this function broken ?
Thank you very much!

Comment 16

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Hi Rachelle,

I think the user impact is not much. The point is the original design is somewhat awkward because "rejecting a call or not" should not be a decision of device.

User will get confused in following STR (but it's rare):
1. On device A, make a MO call to another device, say B
2. Answer the call on device B.
3. On device A, tap home and enter dialer again
4. call the number of B again
5. the phone call made at step 1 is on hold, and device B stays on dialer app, no specific message is shown.

At step 5, user will get confused of this situation (no message shown for call made or not).


Thanks

Comment 17

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Forgot to say that: This bug was originally reported by woodduck partner.

Comment 18

[howie [:howie]]

Triage: blocking 2.0+

Comment 19

[Anthony Ricaud (:rik)]

I don't think this is a 2.0 blocker. But we can ask for approvals (to 2.0 and 2.1).

Comment 20

[Anthony Ricaud (:rik)]

Needinfo-ing Tzu-Lin to ask for approvals.

Comment 21

[Tzu-Lin Huang [:dwi2][:tzhuang]]

This was reported from partner, set 2.0m affected

Comment 22

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Comment on attachment 8494925 [details] [review]
Pull request

NOTE: Please see https://wiki.mozilla.org/Release_Management/B2G_Landing to better understand the B2G approval process and landings.

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
The original design is somewhat awkward because "rejecting a call or not" should not be a decision of device.

User will get confused in following STR:
1. On device A, make a MO call to another device, say B
2. Answer the call on device B.
3. On device A, tap home and enter dialer again
4. call the number of B again
5. the phone call made at step 1 is on hold, and device B stays on dialer app, no specific message is shown.

At step 5, user will get confused of this situation (no message shown for call made or not).

[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
[String changes made]:

Comment 23

[Ryan VanderMeulen [:RyanVM]]

You need to request Gaia v2.1 approval as well (each branch needs its own approval regardless of blocking status).

Comment 24

[Tzu-Lin Huang [:dwi2][:tzhuang]]

Comment on attachment 8494925 [details] [review]
Pull request

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #):
[User impact] if declined:
The original design is somewhat awkward because "rejecting a call or not" should not be a decision of device.

User will get confused in following STR:
1. On device A, make a MO call to another device, say B
2. Answer the call on device B.
3. On device A, tap home and enter dialer again
4. call the number of B again
5. the phone call made at step 1 is on hold, and device B stays on dialer app, no specific message is shown.

At step 5, user will get confused of this situation (no message shown for call made or not).

[Testing completed]:
[Risk to taking this patch] (and alternatives if risky):
very low risk. According to comment 9, with promise architecture, we won't get stuck even if we make second call to the same number too fast
[String changes made]:

Comment 25

[bhavana bajaj [:bajaj]]

Comment on attachment 8494925 [details] [review]
Pull request

This is a real edge case that we've shipped with before and its too late to land on 2.0 now, so I am minusing the request and taking this for 2.1 only.

If am switching the nom to the woodduck branch, if it needs to land there.

Comment 26

[Ryan VanderMeulen [:RyanVM]]

v2.1: https://github.com/mozilla-b2g/gaia/commit/09fae6ff54f5d134207e9be318c56e64bb8dea79
v2.0: https://github.com/mozilla-b2g/gaia/commit/5082fc56ef80352b7bc1be6b934286d815f069d8

Comment 27

[Kai-Zhen Li [:seinlin][:kli]]

v2.0m https://github.com/mozilla-b2g/gaia/commit/5082fc56ef80352b7bc1be6b934286d815f069d8

Comment 28

[Anthony Ricaud (:rik)]

Why has this landed on 2.0 and 2.0m? Comment 25 has not given approval for those branches.

Comment 29

[Ryan VanderMeulen [:RyanVM]]

(In reply to Anthony Ricaud (:rik) from comment #28)
> Why has this landed on 2.0

Because I saw a '+' instead of a '-' when going through the 40+ bug dump of approvals I got yesterday. Reverted.
v2.0:  https://github.com/mozilla-b2g/gaia/commit/944b6421ae7ddd743e1245699461926fcd641528
v2.0m: https://github.com/mozilla-b2g/gaia/commit/944b6421ae7ddd743e1245699461926fcd641528

Of course, it would also have been helpful if the blocking status was updated to reflect v2.0 being wontfixed at the time of comment 25. I've gone ahead and changed the flags based on comment 25.

> and 2.0m?

Kai-Zhen is handling the 2.0->2.0m merges. Note that the hash is the same. He did nothing wrong here.

Comment 30

[Josh Cheng [:josh]]

Nominate this to 2.0M+
This is reported by partner and high lighted they want the fix before 12nd. 

Hi Kai-Zhen,
Could you please land this patch to 2.0M？ 
Thank you！

Comment 31

[Kai-Zhen Li [:seinlin][:kli]]

Ryan, Thanks for helping me clarify it. 

Now patch is landed in 2.0m only.
v2.0m: https://github.com/mozilla-b2g/gaia/commit/76e5b0811032e68c3ad32585c43ea58f37cbf059

Comment 32

[sarsenyev]

Verified, the bug is fixed on 2.2, 2.1 
User is able to make to make an additional call to the same device with the same or a different extension
with option hold or hang up on the first call

Device: Flame 2.2 Master KK
BuildID: 20141028040202
Gaia: 6a7fb482a03c5083ef79b41e7b0dfab27527cd04
Gecko: a255a234946e
Gonk: 6e51d9216901d39d192d9e6dd86a5e15b0641a89
Version: 36.0a1 (2.2 Master)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Device: Flame 2.1 KK
BuildID: 20141028001203
Gaia: a0174f7166745256aaca1cb3aa9f894033fbffa6
Gecko: 43bda3541f6b
Gonk: 6e51d9216901d39d192d9e6dd86a5e15b0641a89
Version: 34.0 (2.1)
Firmware: V188
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0

_____________________________________________________________________
Leaving verifyme keyword for 2.0M. We do not have any device to check on 2.0M.

Comment 33

[Coler]

This issue has been verified successfully on Woodduck 2.0.
Reproducing rate: 0/5
See attachment: Verify_Woodduck_Call.MP4

build version:
Gaia-Rev        87b23fa81c3b59f2ba24b84f7d686f4160d4e7cb
Gecko-Rev       d049d4ef127844121c9cf14d2e8ca91fd9045fcb
Build-ID        20141124050313
Version         32.0
Note:It will prompt "The call is busy now" when call fixed-line telephone at second time;and the call will be connected if we test with mobile phone number.

Comment 34

[Coler]

Attachment: Verify_Woodduc k_Call.MP4