Closed Bug 888799 Opened 8 years ago Closed 8 years ago

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

Categories

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

x86_64
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: aknow, Assigned: tzimmermann)

References

Details

Attachments

(1 file, 1 obsolete file)

'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
Depends on: 881142
Attached file Github pull request (obsolete) —
@l10n: I hope this is right for getting the new message strings reviewed?
Attachment #785791 - Flags: ui-review?(l10n)
Attachment #785791 - Flags: review?(szchen)
Assignee: nobody → tzimmermann
Comment on attachment 785791 [details]
Github pull request

Hi Rex, It's gaia dialer modification. Please help for this. Thank you.
Attachment #785791 - Flags: review?(szchen) → review?(rexboy)
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.
Attachment #785791 - Flags: ui-review?(l10n)
Attachment #785791 - Flags: ui-review?(firefoxos-ux-bugzilla)
(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 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!
Attachment #785791 - Flags: review?(rexboy)
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 on attachment 785791 [details]
Github pull request

No problem, let's ask Etienne.
Attachment #785791 - Flags: review?(etienne)
I updated the pull re
...quest accoridn to the review.
Let's try this again:

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

;)
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.
Attachment #785791 - Flags: review?(etienne)
Hi Etienne,

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

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

Thanks!
Flags: needinfo?(firefoxos-ux-bugzilla)
(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!
Flagging Matej for string review.
Flags: needinfo?(firefoxos-ux-bugzilla) → needinfo?(Mnovak)
Comment on attachment 785791 [details]
Github pull request

Flagging Matej for string review.
Attachment #785791 - Flags: ui-review?(firefoxos-ux-bugzilla) → ui-review?
Attachment #785791 - Flags: ui-review? → ui-review?(Mnovak)
Sorry everybody, I'm a GitHub noob. Can someone let me know which strings I need to review? Thanks.
Flags: needinfo?(Mnovak)
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).
(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.
Attached file Github pull request
I updated the strings according to the suggestions and squashed the patches into one.
Attachment #785791 - Attachment is obsolete: true
Attachment #785791 - Flags: ui-review?(Mnovak)
Attachment #790753 - Flags: review?(etienne)
Comment on attachment 790753 [details]
Github pull request

r=me (d1baa42)
Attachment #790753 - Flags: review?(etienne) → review+
Etienne,

Please merge the pull request. I don't have commit access to the Gaia repo. Thanks!
https://github.com/mozilla-b2g/gaia/commit/8fd7f5656d75f6c7ce73a93addd21e7f33a7943f
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.