888799 - [Dialer] Remove the usage of busy state in telephony call

Status: RESOLVED FIXED

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

Description

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

'busy' state is removed in Bug 881142, but instead signaling through the call error.

Dialer currently refer to busy state here. This part should has some modifications.
https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/handled_call.

call.onerror = function errorCB(evt) {
  if (evt.call.error.name == 'BusyError') busy();
};

Btw, you may also detect user no response/reject through call error.

"UserAlertingNoAnswerError"; <= called ring, user no answer
"CallRejectedError";         <= user reject

Comment 1

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

Correct the link
https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/handled_call.js#L68

Comment 2

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: Github pull request

@l10n: I hope this is right for getting the new message strings reviewed?

Comment 3

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

Comment on attachment 785791 [details]
Github pull request

Hi Rex, It's gaia dialer modification. Please help for this. Thank you.

Comment 4

[Axel Hecht [:Pike]]

Comment on attachment 785791 [details]
Github pull request

We don't do string reviews. If you want to get input on the strings, please reach out to UX. They're using the ui-review flag for that these days. See https://groups.google.com/d/msg/mozilla.dev.gaia/_T_BTvJ4fJk/6RLVb_hFl5UJ for details.

Comment 5

[Thomas Zimmermann [:tzimmermann] [:tdz]]

(In reply to Axel Hecht [:Pike] from comment #4)
> Comment on attachment 785791 [details]
> Github pull request
> 
> We don't do string reviews. If you want to get input on the strings, please
> reach out to UX. They're using the ui-review flag for that these days. See
> https://groups.google.com/d/msg/mozilla.dev.gaia/_T_BTvJ4fJk/6RLVb_hFl5UJ
> for details.

Ok, thanks anyway.

Comment 6

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Comment on attachment 785791 [details]
Github pull request

Hello Aknow:
Just had a look, the main code seems ok to me, but you may need to clean
the remaining unit test code for busy state, so that we won't break the unit
test for dialer. See my comments on Github for detail.
Thank you!

Comment 7

[KM Lee [:rexboy] (inactivate after Sep. 22)]

Comment on attachment 785791 [details]
Github pull request

And also, since I'm not a peer, I think :gtorodelvalle or :Etienne is more appropriate
to review the patch. 
(But I guess German is on PTO? He hasn't on Bugzilla for a while.)

Comment 8

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Comment on attachment 785791 [details]
Github pull request

No problem, let's ask Etienne.

Comment 9

[Thomas Zimmermann [:tzimmermann] [:tdz]]

I updated the pull re

Comment 10

[Thomas Zimmermann [:tzimmermann] [:tdz]]

...quest accoridn to the review.

Comment 11

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Let's try this again:

'I updated the pull request according to the review.'

;)

Comment 12

[Etienne Segonzac (:etienne)]

Comment on attachment 785791 [details]
Github pull request

Comment on github.
The code looks good, but we're now in good enough shape to request a unit test for this change :)

Also we should make sure that this change is not seen as a regression of bug 857951.
Looks like the message is enough but I'd like to confirm this.

Comment 13

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Hi Etienne,

I updated the pull request with a unit test. It's basically the same as for the other error codes.

Comment 14

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Hi UX team,

Please have a look at the string changes in the attached pull request.

Thanks!

Comment 15

[Etienne Segonzac (:etienne)]

(In reply to Thomas Zimmermann [:tzimmermann] [:tdz] from comment #13)
> Hi Etienne,
> 
> I updated the pull request with a unit test. It's basically the same as for
> the other error codes.

Cool.

Can you:
* squash
* rebase on master (the PR is bitrotten)
* flag me for a final r?

Thanks!

Comment 16

[Stephany Wilkes]

Flagging Matej for string review.

Comment 17

[Stephany Wilkes]

Comment on attachment 785791 [details]
Github pull request

Flagging Matej for string review.

Comment 18

[Matej Novak [:matej]]

Sorry everybody, I'm a GitHub noob. Can someone let me know which strings I need to review? Thanks.

Comment 19

[Francesco Lodolo [:flod]]

These are the two strings I see in the PR

calleeIsBusyTitle=Callee is busy
calleeIsBusyMessage=The person you have called is currently busy. Please try again later. 

Personally I found strange the reference to "the person" (I'm calling a number, not necessarily a person).

Comment 20

[Matej Novak [:matej]]

(In reply to Francesco Lodolo [:flod] from comment #19)
> These are the two strings I see in the PR
> 
> calleeIsBusyTitle=Callee is busy
> calleeIsBusyMessage=The person you have called is currently busy. Please try
> again later. 
> 
> Personally I found strange the reference to "the person" (I'm calling a
> number, not necessarily a person).

I agree. And "callee" isn't a very friendly English term. Maybe we could really simplify it:

calleeIsBusyTitle=Number is busy
calleeIsBusyMessage=Please try again later.

Or a longer version:

calleeIsBusyTitle=Number is busy
calleeIsBusyMessage=The number is currently busy. Please try again later.

Comment 21

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Attachment: Github pull request

I updated the strings according to the suggestions and squashed the patches into one.

Comment 22

[Etienne Segonzac (:etienne)]

Comment on attachment 790753 [details]
Github pull request

r=me (d1baa42)

Comment 23

[Thomas Zimmermann [:tzimmermann] [:tdz]]

Etienne,

Please merge the pull request. I don't have commit access to the Gaia repo. Thanks!

Comment 24

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/commit/8fd7f5656d75f6c7ce73a93addd21e7f33a7943f