Last Comment Bug 765231 - Telephony: Implement dialEmergency() to limit call to emergency numbers
: Telephony: Implement dialEmergency() to limit call to emergency numbers
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: DOM: Device Interfaces (show other bugs)
: Trunk
: ARM Gonk (Firefox OS)
: -- normal (vote)
: mozilla17
Assigned To: Hsin-Yi Tsai [:hsinyi]
:
: Andrew Overholt [:overholt]
Mentors:
: 751550 (view as bug list)
Depends on:
Blocks: webtelephony b2g-ril b2g-product-phone
  Show dependency treegraph
 
Reported: 2012-06-15 07:12 PDT by Tim Guan-tin Chien [:timdream] (please needinfo)
Modified: 2012-07-24 09:25 PDT (History)
15 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
+
+


Attachments
patch (8.28 KB, patch)
2012-07-18 23:08 PDT, Hsin-Yi Tsai [:hsinyi]
philipp: review+
Details | Diff | Splinter Review
Patch (v2) (16.26 KB, patch)
2012-07-22 21:16 PDT, Hsin-Yi Tsai [:hsinyi]
philipp: review+
Details | Diff | Splinter Review

Description Tim Guan-tin Chien [:timdream] (please needinfo) 2012-06-15 07:12:56 PDT
per discussion on dev-b2g, when the phone is connected to a voice network, the emergency keypad on lock screen would need to know whether or not the user is indeed dialing an emergency number or not.

ril_worker.js has already contain the method. We would just need to expose it. Thanks.
Comment 1 José Antonio Olivera Ortega [:jaoo] 2012-06-15 11:23:11 PDT
Is anyone already working on this? If not, I would like to take it and start working on it because I have been working on all the stuff related to emergency calls in B2G.
Comment 2 Philipp von Weitershausen [:philikon] 2012-06-15 11:28:31 PDT
Go for it!

Note that if we want to make this a synchronous call, we will have to cache the emergency number list at least on the chrome process main thread.
Comment 3 Mounir Lamouri (:mounir) 2012-06-18 05:51:09 PDT
I'm just back from vacation so I wasn't able to comment on dev-b2g before but I think this issue should have been raised in dev-webapi actually. The main issue being how to expose the information in the DOM API.

What are the situations where we want to do emergency calls? On top of my heads, I see two:
- no SIM card (or not activated/valid/anything);
- screen locked.

The RIL backend knows about the SIM card status so the call can just be made as usual and would fail. No need for the DOM API to provide any more information except maybe a clear DOMError message.
For the second situation, assuming we actually want to allow emergency calls when the screen is locked, I would prefer if we could know internally or trough an API if making calls is allowed. I guess the lock screen is part of the homescreen app?
If we can't know that information internally, I would prefer a callEmergency() number because it will reduce the leak of personal information via that API. Basically, isEmergencyCall() might allow an application to know in which country you are without requesting SIM or network information. I would prefer if we could prevent that.

(I will try to reply to the thread, better to have that discussion in the mailing-list.)
Comment 4 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-06-18 08:54:39 PDT
@mounir Thank you for your analytical insight. In the discussion on dev-b2g, main reason that favor isEnergencyNumber() over callEmergency() is because the former method allow content web applications to "gray out" the dial button before the user hitting it. The later method will prevent web authors from making such UI, since the app would always have to actually make the call to know if it's a real number.

Your concern on privacy is valid, however, it's not impossible to leak personal info with callEmergency(). The app would just have to call out and hang up quickly -- if the error return faster than the hang up, then it's an invalid number.

Security and privacy trumps UI design, but if neither proposal is equally risky then we pick the useful one.

Keep in mind any web app that gets mozTelephony API is likely a certified and trusted app -- if we trust the app to dial any phone number for us, why would we afraid of it knowing the emergency number?
Comment 5 fabiohmagnoni 2012-06-19 08:05:31 PDT
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #1)
> Is anyone already working on this? If not, I would like to take it and start
> working on it because I have been working on all the stuff related to
> emergency calls in B2G.

José, can I help you to work on it? I have a lot of interest in this part.
Comment 6 Mounir Lamouri (:mounir) 2012-06-19 09:26:59 PDT
(In reply to Tim Guan-tin Chien [:timdream] (MoCo-TW) from comment #4)
> @mounir Thank you for your analytical insight. In the discussion on dev-b2g,
> main reason that favor isEnergencyNumber() over callEmergency() is because
> the former method allow content web applications to "gray out" the dial
> button before the user hitting it. The later method will prevent web authors
> from making such UI, since the app would always have to actually make the
> call to know if it's a real number.

Not being able to do so doesn't seem that bad.

> Your concern on privacy is valid, however, it's not impossible to leak
> personal info with callEmergency(). The app would just have to call out and
> hang up quickly -- if the error return faster than the hang up, then it's an
> invalid number.

IMO, a call shouldn't be placed without user interaction so that would be really hard to do. In addition, it seems impossible for an application to do that without being caught which means the user will uninstall it.

> Keep in mind any web app that gets mozTelephony API is likely a certified
> and trusted app -- if we trust the app to dial any phone number for us, why
> would we afraid of it knowing the emergency number?

We shouldn't design an API assuming it will be used only in a specific context. What you should assume is that WebTelephony will be -likely- restricted, for B2G v1, for certified or trusted apps. However, that doesn't mean any app shoudn't be allowed to use the API someday. Designing an API assuming it will be used on a restricted part of the web isn't the Web.
Also, even if I install an application I trust, that doesn't mean I would trust the application not to try to fingerprint me, track me and get information about me for targeted ads. Any application with in-app advertisement or with an advertisement network will be willing to do so. By giving the right for an application to make and take phone calls I will assume the application will know who I call and who calls me. I'm not expecting it to know in which country I am.
Comment 7 Philipp von Weitershausen [:philikon] 2012-06-19 13:55:09 PDT
(In reply to Mounir Lamouri (:mounir) from comment #3)
> For the second situation, assuming we actually want to allow emergency calls
> when the screen is locked, I would prefer if we could know internally or
> trough an API if making calls is allowed. I guess the lock screen is part of
> the homescreen app?

Are you saying you want the RIL to know whether the screen is locked or not? I feel like that's slightly the wrong question that the RIL wants to ask. How about we expose a flag, e.g. "onlyAllowEmergencyCalls", that the homescreen app can set to true whenever the screen is locked?

(In reply to Mounir Lamouri (:mounir) from comment #6)
> (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TW) from comment #4)
> > @mounir Thank you for your analytical insight. In the discussion on dev-b2g,
> > main reason that favor isEnergencyNumber() over callEmergency() is because
> > the former method allow content web applications to "gray out" the dial
> > button before the user hitting it. The later method will prevent web authors
> > from making such UI, since the app would always have to actually make the
> > call to know if it's a real number.
> 
> Not being able to do so doesn't seem that bad.

I agree.

I would be ok with introducing a dialEmergency() method (I would recommend not using the "call" word since so far the API uses the "dial".)
Comment 8 Mounir Lamouri (:mounir) 2012-06-20 09:09:44 PDT
(In reply to Philipp von Weitershausen [:philikon] from comment #7)
> (In reply to Mounir Lamouri (:mounir) from comment #3)
> > For the second situation, assuming we actually want to allow emergency calls
> > when the screen is locked, I would prefer if we could know internally or
> > trough an API if making calls is allowed. I guess the lock screen is part of
> > the homescreen app?
> 
> Are you saying you want the RIL to know whether the screen is locked or not?
> I feel like that's slightly the wrong question that the RIL wants to ask.

Never said that. RIL should know if "making calls is allowed".

> How about we expose a flag, e.g. "onlyAllowEmergencyCalls", that the
> homescreen app can set to true whenever the screen is locked?

That or something else. I don't know. I don't really like the idea of adding such a flag in WebTelephony. The homescreen should likely know that information. We could maybe use the hacky MozChromeEvent to pass that information to the chrome. We definitely need that Homescreen API...

> (In reply to Mounir Lamouri (:mounir) from comment #6)
> > (In reply to Tim Guan-tin Chien [:timdream] (MoCo-TW) from comment #4)
> > > @mounir Thank you for your analytical insight. In the discussion on dev-b2g,
> > > main reason that favor isEnergencyNumber() over callEmergency() is because
> > > the former method allow content web applications to "gray out" the dial
> > > button before the user hitting it. The later method will prevent web authors
> > > from making such UI, since the app would always have to actually make the
> > > call to know if it's a real number.
> > 
> > Not being able to do so doesn't seem that bad.
> 
> I agree.
> 
> I would be ok with introducing a dialEmergency() method (I would recommend
> not using the "call" word since so far the API uses the "dial".)

Ok for dial instead of call but ideally I would prefer if we could prevent that to be a method and enforce the call to be made only to emergency numbers.

I don't have a strong opinion for specific method VS internal state. If anyhow has thoughts on this..?
Comment 9 Philipp von Weitershausen [:philikon] 2012-06-20 09:32:10 PDT
(In reply to Mounir Lamouri (:mounir) from comment #8)
> (In reply to Philipp von Weitershausen [:philikon] from comment #7)
> > (In reply to Mounir Lamouri (:mounir) from comment #3)
> > > For the second situation, assuming we actually want to allow emergency calls
> > > when the screen is locked, I would prefer if we could know internally or
> > > trough an API if making calls is allowed. I guess the lock screen is part of
> > > the homescreen app?
> > 
> > Are you saying you want the RIL to know whether the screen is locked or not?
> > I feel like that's slightly the wrong question that the RIL wants to ask.
> 
> Never said that. RIL should know if "making calls is allowed".

Fair enough. How are you proposing the RIL will know this if it's not trough a flag (like the `onlyAllowEmergencyCalls` thing I proposed)?

> > I would be ok with introducing a dialEmergency() method (I would recommend
> > not using the "call" word since so far the API uses the "dial".)
> 
> Ok for dial instead of call but ideally I would prefer if we could prevent
> that to be a method and enforce the call to be made only to emergency
> numbers.

Ok so you're saying you want to use the existing dial() method we have even for emergency calls, but somehow the RIL is informed that only emergency calls should be allowed, so it will reject any other numbers?
Comment 10 Mounir Lamouri (:mounir) 2012-06-20 09:40:09 PDT
(In reply to Philipp von Weitershausen [:philikon] from comment #9)
> (In reply to Mounir Lamouri (:mounir) from comment #8)
> > (In reply to Philipp von Weitershausen [:philikon] from comment #7)
> > > (In reply to Mounir Lamouri (:mounir) from comment #3)
> > > > For the second situation, assuming we actually want to allow emergency calls
> > > > when the screen is locked, I would prefer if we could know internally or
> > > > trough an API if making calls is allowed. I guess the lock screen is part of
> > > > the homescreen app?
> > > 
> > > Are you saying you want the RIL to know whether the screen is locked or not?
> > > I feel like that's slightly the wrong question that the RIL wants to ask.
> > 
> > Never said that. RIL should know if "making calls is allowed".
> 
> Fair enough. How are you proposing the RIL will know this if it's not trough
> a flag (like the `onlyAllowEmergencyCalls` thing I proposed)?
> 
> > > I would be ok with introducing a dialEmergency() method (I would recommend
> > > not using the "call" word since so far the API uses the "dial".)
> > 
> > Ok for dial instead of call but ideally I would prefer if we could prevent
> > that to be a method and enforce the call to be made only to emergency
> > numbers.
> 
> Ok so you're saying you want to use the existing dial() method we have even
> for emergency calls, but somehow the RIL is informed that only emergency
> calls should be allowed, so it will reject any other numbers?

Yes. That or a dialEmergency() method.
Comment 11 Philipp von Weitershausen [:philikon] 2012-06-29 17:33:44 PDT
This is a blocker and needs an owner. Hsinyi and Price, can one of you please take this? Thanks!


(In reply to Mounir Lamouri (:mounir) from comment #10)
> (In reply to Philipp von Weitershausen [:philikon] from comment #9)
> > (In reply to Mounir Lamouri (:mounir) from comment #8)
> > > (In reply to Philipp von Weitershausen [:philikon] from comment #7)
> > > > (In reply to Mounir Lamouri (:mounir) from comment #3)
> > > > > For the second situation, assuming we actually want to allow emergency calls
> > > > > when the screen is locked, I would prefer if we could know internally or
> > > > > trough an API if making calls is allowed. I guess the lock screen is part of
> > > > > the homescreen app?
> > > > 
> > > > Are you saying you want the RIL to know whether the screen is locked or not?
> > > > I feel like that's slightly the wrong question that the RIL wants to ask.
> > > 
> > > Never said that. RIL should know if "making calls is allowed".
> > 
> > Fair enough. How are you proposing the RIL will know this if it's not trough
> > a flag (like the `onlyAllowEmergencyCalls` thing I proposed)?

Mounir, you never answered this question. Could you let us know how you would like that question solved? I think then this bug should be good to go for somebody to implement.
Comment 12 Mounir Lamouri (:mounir) 2012-06-30 10:00:26 PDT
We can add a dialEmergency() method to the Telephony API that will fail asynchrously if the dialed number isn't an emergency number.
Comment 13 Gene Lian [:gene] (I already quit Mozilla) 2012-07-05 21:02:37 PDT
Hi guys,

Is there anyone attempting to taking this so far? If not, may I take this? :) I've already gone through the discussion thread and it makes sense to me. Please feel free to let me know if I'm suitable to take this or not.

Thanks,
Gene
Comment 14 Hsin-Yi Tsai [:hsinyi] 2012-07-05 21:23:48 PDT
(In reply to Gene Lian [:gene] from comment #13)
> Hi guys,
> 
> Is there anyone attempting to taking this so far? If not, may I take this?
> :) I've already gone through the discussion thread and it makes sense to me.
> Please feel free to let me know if I'm suitable to take this or not.
> 
> Thanks,
> Gene

Hi Gene,

I discussed with Price about Comment #11 this Monday. How about we waiting for Price's response?
Comment 15 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-07-05 21:25:41 PDT
(In reply to Mounir Lamouri (:mounir) from comment #12)
> We can add a dialEmergency() method to the Telephony API that will fail
> asynchrously if the dialed number isn't an emergency number.

I don't think the UX benefit from isEmergencyNumber() than dialEmergency() is really a must-have feature, but Josh is going to check with other UX on that. I think we can implement dialEmergency() if UX does not have strong opinion on that.
Comment 16 Mounir Lamouri (:mounir) 2012-07-06 02:02:09 PDT
Sicking and I agreed that dialEmergency() is a better API than isEmergencyNumber(). I think we should go with that.
Comment 17 Philipp von Weitershausen [:philikon] 2012-07-06 21:09:11 PDT
Great!

This is a blocker without an owner. Price, can you take this? If not, let me know. Thanks!
Comment 18 Josh Carpenter [:jcarpenter] 2012-07-09 04:05:34 PDT
Following up for UX: Tim put the question to me as follows:

"do we want to check the number dialed in and only enable the dial button if the user dialed a valid emergency number, or we don't check it at all and allow the backend to return error if it's not a emergency number?"

Can you help me to understand the UX flow for the later situation? eg: User enters number, hits "Call" button. If number entered is not a valid emergency number for the calling area, system sounds error audio alert and displays UI prompt (eg: "this is not an emergency number").

If that's the flow, I'm fine with it.
Comment 19 Philipp von Weitershausen [:philikon] 2012-07-09 16:38:44 PDT
(In reply to Josh Carpenter from comment #18)
> "do we want to check the number dialed in and only enable the dial button if
> the user dialed a valid emergency number, or we don't check it at all and
> allow the backend to return error if it's not a emergency number?"

Checking ahead of time is tedious and it's questionable whether it's worth the effort.

> Can you help me to understand the UX flow for the later situation? eg: User
> enters number, hits "Call" button. If number entered is not a valid
> emergency number for the calling area, system sounds error audio alert and
> displays UI prompt (eg: "this is not an emergency number").

This flow we can implement.
Comment 20 Josh Carpenter [:jcarpenter] 2012-07-09 18:14:16 PDT
Works for me :)
Comment 21 Hsin-Yi Tsai [:hsinyi] 2012-07-18 23:08:22 PDT
Created attachment 643730 [details] [diff] [review]
patch

Implement dialEmergency() and return 'callError' if not a valid emergency number.
Comment 22 Philipp von Weitershausen [:philikon] 2012-07-19 13:44:45 PDT
Comment on attachment 643730 [details] [diff] [review]
patch

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

::: dom/telephony/Telephony.cpp
@@ +256,5 @@
>    return NS_OK;
>  }
>  
>  NS_IMETHODIMP
> +Telephony::DialEmergency(const nsAString& aNumber, nsIDOMTelephonyCall** aResult)

Instead of duplicating this code, I would suggest creating a

  Telephony::DialInternal(bool isEmergency, const nsAString& aNumber, nsIDOMTelephonyCall** aResult)

helper and then calling that from `Telephony::Dial()` and `Telephony::DialEmergency()``.

Also, is there a way to test this stuff with the emulator? If so, it'd be nice to have a Marionette test.

r=me with that.
Comment 23 Hsin-Yi Tsai [:hsinyi] 2012-07-19 18:43:46 PDT
(In reply to Philipp von Weitershausen [:philikon] from comment #22)
> Comment on attachment 643730 [details] [diff] [review]
> patch
> 
> Review of attachment 643730 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: dom/telephony/Telephony.cpp
> @@ +256,5 @@
> >    return NS_OK;
> >  }
> >  
> >  NS_IMETHODIMP
> > +Telephony::DialEmergency(const nsAString& aNumber, nsIDOMTelephonyCall** aResult)
> 
> Instead of duplicating this code, I would suggest creating a
> 
>   Telephony::DialInternal(bool isEmergency, const nsAString& aNumber,
> nsIDOMTelephonyCall** aResult)
> 
> helper and then calling that from `Telephony::Dial()` and
> `Telephony::DialEmergency()``.
> 
Good suggestion. I'll do that in an update. Thanks.

> Also, is there a way to test this stuff with the emulator? If so, it'd be
> nice to have a Marionette test.
> 
> r=me with that.
I'll investigate it!
Comment 24 Hsin-Yi Tsai [:hsinyi] 2012-07-22 21:16:07 PDT
Created attachment 644829 [details] [diff] [review]
Patch (v2)

Patch: update mActiveCall and add test cases

Hi Philipp,
Test cases and DialInternal() are provided according to your suggestions. I also updated mActiveCall in 'NotifyError()' to make sure the telephony logic. Thanks.
Comment 25 Philipp von Weitershausen [:philikon] 2012-07-23 17:50:49 PDT
Comment on attachment 644829 [details] [diff] [review]
Patch (v2)

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

Excellent work! Tests <3 <3 <3
Comment 26 Hsin-Yi Tsai [:hsinyi] 2012-07-23 19:42:08 PDT
*** Bug 751550 has been marked as a duplicate of this bug. ***
Comment 28 Ed Morley [:emorley] 2012-07-24 02:57:19 PDT
https://hg.mozilla.org/mozilla-central/rev/c500850abf3f
Comment 29 Tim Guan-tin Chien [:timdream] (please needinfo) 2012-07-24 09:25:37 PDT
@hsinyi Thank you!

Note You need to log in before you can comment on or make changes to this bug.