Closed Bug 856074 Opened 10 years ago Closed 10 years ago

Controls on the call screen should be disabled until the call is reported connecting when dialing emergency number in airplane mode

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set
normal

Tracking

(blocking-b2g:leo+, b2g18 fixed)

RESOLVED FIXED
blocking-b2g leo+
Tracking Status
b2g18 --- fixed

People

(Reporter: anshulj, Assigned: gtorodelvalle)

References

Details

Attachments

(4 files)

STR:
Prerequisites are that fixes for bug 839363 and bug 849185 should be available in the build.

1. Enable Airplane mode
2. Make an emergency call from the dialer
3. Before gecko could place the emergency call, press the end key.

Observed behavior: The oncall screen goes away but no RIL:Hangup request is sent to gecko. So gecko continues to place the call after airplane mode is turned off and signal is available.

Expected Behavior: I think the possible fix here could be to keep the disconnect option disabled until the call is reported dialing. Possibly need to involve UX folks.
can we get a video of this issue?
Also was this done on the ril build or Mozril?
Flags: needinfo?(anshulj)
Jamie can you weigh in on the expected behaviour here?
Flags: needinfo?(jachen)
Wayne, this was done with commercial RIL but I can easily reproduce this issue with Moz RIL. Will upload the video shortly.
Flags: needinfo?(anshulj)
:etienne - Is this something you can check from dialer first?
Flags: needinfo?(etienne)
I have uploaded a video at http://youtu.be/LM4FhEp75dg. There are two issues I have captured.

1. As soon as the airplane mode is enabled and before the call is actually connected, if I press the hangup button, the RIL:Hangup request is not sent down to gecko so when the voice service is available the telephony code places the call and that is when you would see the call screen showing up again. I think we should disable the hangup button until the call is actually connected (this is how Android behaves).

2. I see an incoming call like UI with an answer button and an end button instead of outgoing call UI.

I pulled in the patch at bug 839363 to enable the UI to place an emergency call. I wasn't able to reproduce the scenario with Moz RIL as with Moz RIL dial request itself is failing.
(In reply to Anshul from comment #5)
> I have uploaded a video at http://youtu.be/LM4FhEp75dg. There are two issues
> I have captured.
> 
> 1. As soon as the airplane mode is enabled and before the call is actually
> connected, if I press the hangup button, the RIL:Hangup request is not sent
> down to gecko so when the voice service is available the telephony code
> places the call and that is when you would see the call screen showing up
> again. I think we should disable the hangup button until the call is
> actually connected (this is how Android behaves).
> 
> 2. I see an incoming call like UI with an answer button and an end button
> instead of outgoing call UI.
> 
> I pulled in the patch at bug 839363 to enable the UI to place an emergency
> call. I wasn't able to reproduce the scenario with Moz RIL as with Moz RIL
> dial request itself is failing.

Interesting... looks like by the time the first call screen is fully loaded the call is not there yet. And by the time the call does come in the call screen is already closed so we open a new one.

Maybe we should add a bit of logic in the call screen where, if the radio is disabled, we wait for the settingchange event telling us it's enabled before displaying the controls.
(Android does something similar I think where they show an "activating GSM..." message.)

But of course if we do this we need to make sure that the call screen never gets open in cases where the radio is disabled and will stay disabled.
Flags: needinfo?(etienne)
Eitenne, I see the same issue when dialing an emergency number through the emergency dialer in the phone lock screen in airplane mode. So would you be working on this bug?

Also, the fix https://github.com/mozilla-b2g/gaia/pull/8887 for bug 850119 doesn't even let user make a call when in APM.
Assignee: nobody → etienne
blocking-b2g: leo? → leo+
Summary: Call hangup request not being sent to gecko when dialing emergency number in airplane mode → Controls on the call screen should be disabled until the call is reported connecting when dialing emergency number in airplane mode
https://bugzilla.mozilla.org/show_bug.cgi?id=856074#c7

I'm concerned about the bug listed in that comment doesn't allow user to make an emergency call in APM.

I'm fine that the controls should be disabled until the call is reported connected.
Flags: needinfo?(jachen) → needinfo?(anshulj)
Flags: needinfo?(anshulj)
Steal it ;)

I can reproduce this on master today.
Assignee: etienne → yurenju.mozilla
Attached patch WIP patchSplinter Review
WIP patch here.

Etienne, could you give some feedback for this change? looks not so smooth, maybe phone is busy for dialing.
Attachment #745093 - Flags: feedback?(etienne)
Comment on attachment 745093 [details] [diff] [review]
WIP patch

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

Looks like a hidden |setTimeout(, 200)| :)
And it's delaying the opening of the call screen, which we should definitely avoid.

I'm not in favor of adding an arbitrary timeout for this, but if we had too it would be better to just enable the hang up button after a timeout rather than delaying the whole call screen.

This looks like one of the numerous bug that would benefit from bug 823958 being fixed.
Attachment #745093 - Flags: feedback?(etienne) → feedback-
Depends on: 823958
I moved toggleScreen from setup() to onCallsChanged() because I found another problem which is we show two different layouts when emergency call. first one have two buttons, second only have one button, please see this:

https://docs.google.com/a/mozilla.com/file/d/0B-FHjXny08MtTTJSWGR6cG9qNEU/edit

root cause is we take too long time from disable airplane mode to dial emergency call. that's why I wrote this patch for sovling two problems in one commit...

but you are right, I should just disable hang up button until addCall (or a timeout).
I will file a follow-up bug for two different layouts when emergency call.
Attachment #745775 - Flags: review?(etienne)
Depends on: 868920
Comment on attachment 745775 [details]
pull request: https://github.com/mozilla-b2g/gaia/pull/9566

We can't do this without risking completely blocking the phone (until reboot) in cases like bug 860614.

We _always_ need to let the user close the call screen "no questions asks" since, without bug 823958, we can't guarantee that the call screen being open strictly means we have at least one call.
Attachment #745775 - Flags: review?(etienne) → review-
un-assigned this because I'm working on unpredictably ceritification bugs for cusotmization.
Assignee: yurenju.mozilla → nobody
Hi guys! I was considering assigning this bug to me but I am not able to reproduce it. The point is that having a look at the code, I see no reason why I am not able to reproduce it but for timing ones. This is, I do not get the "Airplane mode" deactivated and the attention screen shown so I can hang up the call before Gecko has already established the call. Am I missing anything here? Or do we just want to protect the Dialer from the case that I do not manage to reproduce from happening? Thanks!
Flags: needinfo?(etienne)
BTW, I even deleted the 0.5s from the animation to try to speed the showing of the attention screen but no even in this case I am able to get the attention screen shown after the airplane mode has been deactivated but before Gecko has established the call :-(
I tested again with the latest gecko/gaia and I still see both the issues I reported in comment #5. Although issue #1 reported in comment #5 is not reproducible every single time now. I still see the issue #2. I have attached the video of the issue with commercial RIL. With Moz RIL the DIAL request is failing so I am not able to reproduce the issue.
Geez! That's a kind of long emergency number you have around there :-O

BTW, this is the behavior I am getting right now (latest Gecko and Gaia) in Spain: https://www.dropbox.com/s/jzkff4k9qmo65nw/2013-05-30%2010.10.25.mp4 (the size of the video was bigger than the one Bugzilla allows for attachments :-( )
Apart from this, in your video the animations shown to move from the Settings app to the home screen and then to the Communications app does not seem to be the latest ones :-O Please, no offense but are you sure you are using latest Gecko and Gaia? :-) Or the video you just attached is an old one but the behavior persists using latest Gecko and Gaia :-)
Hi all, I´ve tested in a unagi device with Gecko-517ec8d.Gaia-6849f19 from master and I´m getting the same behaviour described in comment #21. If you hang up fast enough, before the call is connected, device is rebooted as well.
I still see the same issue as I reported in comment #20 with the tip of gecko and gaia from 1.1.

gecko: 1287520f4ae5ef7711a7e8ca307ebbe3c2f0fdaf
gaia: 5919649b3ab60759ad2b55dc7e38b4cfaec7d7b9

Could you please try with commercial RIL. I suspect the issue could be the timing as commercial RIL starts the dial request only after there is a confirmation that the call can be made unlike moz RIL which might attempt to dial a call sooner.
Hi guys! and specially to Anshul ;-) You are right, using Gecko-b7e2734.Gaia-e1c59ba from 1.1 I am able to get the same behavior as you. Thank you very much for your comments!

Taking into consideration the solution provided to bug 823958 (although it appears reopen due to some regression issues if I am not wrong), I would say that it would be enough to move the toggle screen code from https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/oncall.js#L244 to https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/js/oncall.js#L364 (just before the closing '}'). This way we will be showing the attention screen with the right rendering and once the user can act upon it (to hang it up or whatever). I have tested it and it works and the showing of the attention screen does not seem to be penalized (regarding the time taken until it is shown).

As suggested by Etienne, another option would be to show the attention screen ASAP (leaving the toggle screen code where it is now) including disabled controls and to show the final rendering (and controls) once the callschanged is notified and the call is added (addCall()). But for this I guess we need some magic UX touch and help ;-)

Etienne, would you be still in favor of the second option or would you like to try the first one first (I could prepare a PR)? :-)
Ups, forgot to request info from Etienne ;-) Thanks!
Assignee: nobody → gtorodelvalle
(In reply to gtorodelvalle from comment #25)
> Hi guys! and specially to Anshul ;-) You are right, using
> Gecko-b7e2734.Gaia-e1c59ba from 1.1 I am able to get the same behavior as
> you. Thank you very much for your comments!
> 
> Taking into consideration the solution provided to bug 823958 (although it
> appears reopen due to some regression issues if I am not wrong), I would say
> that it would be enough to move the toggle screen code from
> https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/
> js/oncall.js#L244 to
> https://github.com/mozilla-b2g/gaia/blob/master/apps/communications/dialer/
> js/oncall.js#L364 (just before the closing '}'). This way we will be showing
> the attention screen with the right rendering and once the user can act upon
> it (to hang it up or whatever). I have tested it and it works and the
> showing of the attention screen does not seem to be penalized (regarding the
> time taken until it is shown).
> 
> As suggested by Etienne, another option would be to show the attention
> screen ASAP (leaving the toggle screen code where it is now) including
> disabled controls and to show the final rendering (and controls) once the
> callschanged is notified and the call is added (addCall()). But for this I
> guess we need some magic UX touch and help ;-)
> 
> Etienne, would you be still in favor of the second option or would you like
> to try the first one first (I could prepare a PR)? :-)

Thanks to the incredible work on bug 823958 we're now guaranteed to get |onCallsChanged()| called at least once.

So at the end of the |onCallsChanged()| function we can now:
- toggle the callscreen if we have some handledCalls and the screen is not displayed yet
- close the attention screen if we don't have any handledCalls (we're already doing that).

Sounds good?
Flags: needinfo?(etienne)
Attached file Associated PR.
Attachment #757863 - Flags: review?(etienne)
(In reply to gtorodelvalle from comment #25)
> I have tested it and it works and the
> showing of the attention screen does not seem to be penalized (regarding the
> time taken until it is shown).
> 

Wow.
Did some testing with the last patch and I'm having very different results...
Looks like it's adding 1 or 2 rings of delay.

I'll try with a more recent gecko. But we may need to profile this a bit more rigorously, and potentially rethink the solution.
(In reply to Etienne Segonzac (:etienne) from comment #29)
> But we may need to profile this a bit
> more rigorously

The first result are actually very good, we're opening the call screen _sooner_ with your patch :)

(I think my weird delay comes from the SIP phone I'm using to call the B2G phone, digging...)
Comment on attachment 757863 [details]
Associated PR.

Sorry for delaying the review.

Did some thorough testing and this works wonderfully, thanks :)
Attachment #757863 - Flags: review?(etienne) → review+
https://github.com/mozilla-b2g/gaia/commit/d579fe58cf38b1d83dfd87aaca43988a8bb25037
Status: NEW → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Uplifted d579fe58cf38b1d83dfd87aaca43988a8bb25037 to:
v1-train: c0e93b4f0ad432b8b1f69e71f0a86b62e580cb26
Etienne, I tested this patch and it works very well. The issue is resolved. Thanks for your support.
You need to log in before you can comment on or make changes to this bug.