863281 - B2G WebTelephony - expose emergency call info

Status: VERIFIED FIXED

(Firefox OS Graveyard :: General, defect)

Description

[David Palomino [:dpalomino]]

Raised during certification iot, but not a blocking issue

Buildid "20130321070205", device: ikura
gecko commit: b5183c99228bdc5be33340e359efd1b4f0859e92 
gaia commit: 577d13088ebdbd353d13910d3317e713a140415b

When performing a emergency call (for instance, to 112 or 911), a message should be shown to the user noticing that an emergency call is ongoing. 

Suggestions for English/Spanish tags:
English tag: "Emergency call"
Spanish tag: "Llamada de emergencia"

Comment 1

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

Hi Mounir,

For this feature, we have to provide the information for gaia to know whether the call is an emergency call. I learned that this kind of ability has been discussed before. Is there any concern for revealing the information?

Currently, we have two proposals:
1. Add a property field (isEmergency) in telephony call. Thus when making a call, we know its type and could show the correct label.
2. Provide a api to check whether a number is an emergency number. This may become a phone number utility.

I would like to know is there any other user stories related to checking emergency number. This could be very help for designing a proper API.

Comment 2

[David Palomino [:dpalomino]]

Hi, 

We've achieved to decrease priority of this issue with the carrier, so now it is an important issue, but not blocking for certification. 

If it's a risky or difficult issue, I'd suggest to move to tef-. 

Thanks!
David

Comment 3

[Mounir Lamouri (:mounir)]

David, what is the rationale behind that feature? Do people really need to have a label saying that they are doing an emergency call when they actually typed the number? Is that a common UI? Why was that related to certification?

Comment 4

[Juan Perez-Bedmar [:juanpbf]]

Hi :mounir

This feature, is a common UI in handsets.
As part of the certification process, operators usually have a set of test cases related to UI and User Experience, and they give a lot of importance to this part of the testing, since a bad UEx could lead to several complaints on their Call Centers.

Regarding this specific issue, a tag that confirms the users that what they're dialing is, actually, an emergency number, gives them confidence about the action they're performing, confirming they are really dialing an emergency number and not other.

BR

Comment 5

[Mounir Lamouri (:mounir)]

I think we could add a .emergency boolean on the Call interface.

Comment 6

[Lukas Blakk [:lsblakk] use ?needinfo]

We believe that emergency call functionality is important, so definitely tracking, but since it's not a certification blocker we won't block.

Comment 7

[Sarah Parsons]

This issue reproduces on the Inari device. 

Build ID: 20130426070207
Environmental  Variables:
Kernel Date: Feb 21
Gecko: http://hg.mozilla.org/releases/mozilla-b2g18_v1_0_1/rev/54285d67454b
Gaia: c47ef39de04e634d847cc86b6acc616fabce69eb

A message is not shown that the user is connected to an emergency number (911).

Comment 8

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

Attachment: [PATCH 1/4] Add emergency property in TelephonyCall

Comment 9

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

Attachment: [PATCH 2/4] Pass emergency info from ril to dom

Comment 10

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

Attachment: [PATCH 3/4] Update emergency property for outgoing call

Comment 11

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

Current patch has a problem in marionette test.

The solution set the emergency flag in ril_worker only when it enter RIL.CALL_STATE_DIALING. However, in emulator, it seems that RIL will skip this state and directly go into CALL_STATE_ALERTING.

Still need some modification.

Comment 12

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

Attachment: [PATCH 4/4] Add test for emergency label

Comment 13

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

(In reply to Szu-Yu Chen [:aknow] from comment #11)
> Current patch has a problem in marionette test.
> 
> The solution set the emergency flag in ril_worker only when it enter
> RIL.CALL_STATE_DIALING. However, in emulator, it seems that RIL will skip
> this state and directly go into CALL_STATE_ALERTING.
> 
> Still need some modification.

Problem fixed after sync to latest emulator code

Comment 14

[Mounir Lamouri (:mounir)]

Comment on attachment 743005 [details] [diff] [review]
[PATCH 1/4] Add emergency property in TelephonyCall

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

::: dom/telephony/nsITelephonyProvider.idl
@@ +3,4 @@
>   * You can obtain one at http://mozilla.org/MPL/2.0/. */
>  
>  #include "nsISupports.idl"
> +[scriptable, uuid(bde19ba2-83c8-4df7-a014-28fe71535527)]

nit: could you keep the empty line between the "#include" line and the "[scriptable, ...]" one?

Comment 15

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

It's good on try.
https://tbpl.mozilla.org/?tree=Try&rev=1464a5f90428

Comment 16

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

Attachment: [PATCH 1/4 #2] Add emergency property in TelephonyCall

Comment 17

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 743009 [details] [diff] [review]
[PATCH 2/4] Pass emergency info from ril to dom

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

::: dom/system/gonk/ril_worker.js
@@ +3263,5 @@
>          if (newCall.state == CALL_STATE_INCOMING) {
>            newCall.direction = 'incoming';
>          } else if (newCall.state == CALL_STATE_DIALING) {
>            newCall.direction = 'outgoing';
> +          if (!international) newCall.isEmergency =

nit: coding style, use braces.

if (!international) {
  newCall.isEmergency = this._isEmergencyNumber(newCall.number);
}

Comment 18

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 743010 [details] [diff] [review]
[PATCH 3/4] Update emergency property for outgoing call

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

The DOM changes look good to me. Since I am not a DOM peer, ask for DOM peers' review. Need BT peer's review as well.

Comment 19

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 743511 [details] [diff] [review]
[PATCH 4/4] Add test for emergency label

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

The test looks good, but I would like to check 'emergency' in every call state. Let's do that. Thanks!

Comment 20

[Hsin-Yi Tsai (she/her) [:hsinyi]]

B.t.w. please describe the part and version numbers of each attachment clearly in the blank 'Description.' Thanks.

Comment 21

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

Attachment: [PATCH 4/4 #2] Add test for emergency label

Comment 22

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 744469 [details] [diff] [review]
[PATCH 4/4 #2] Add test for emergency label

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

Nice! Thank you.

P.s. don't forget to add 'r=XXX' in the commit comment.

Comment 23

[Gina Yeh [:gyeh] [:ginayeh]]

Comment on attachment 743010 [details] [diff] [review]
[PATCH 3/4] Update emergency property for outgoing call

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

Looks good to me for Bluetooth Hfp part.

Comment 24

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 744408 [details] [diff] [review]
[PATCH 1/4 #2] Add emergency property in TelephonyCall

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

::: dom/telephony/nsITelephonyProvider.idl
@@ +24,5 @@
>    void callStateChanged(in unsigned long callIndex,
>                          in unsigned short callState,
>                          in AString number,
> +                        in boolean isActive,
> +                        in boolean isEmergency);

Sorry, my bad. We should also add |isEmergency| in enumerateCallState. Please take care of this in this and other parts, Aknow.

Comment 25

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

Attachment: [PATCH 1/4 #3] Add emergency property in TelephonyCall.

Comment 26

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

Attachment: [PATCH 2/4 #2] Pass emergency info from ril to dom.

Comment 27

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

Attachment: [PATCH 3/4 #2] Update emergency property for outgoing call.

Comment 28

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

Attachment: [PATCH 1/4 #4] Add emergency property in TelephonyCall.

Comment 29

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

Attachment: [PATCH 2/4 #3] Pass emergency info from ril to dom.

Comment 30

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

Attachment: [PATCH 3/4 #3] Update emergency property for outgoing call.

Comment 31

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

https://tbpl.mozilla.org/?tree=Try&rev=f202ad15b6d4

Comment 32

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 747753 [details] [diff] [review]
[PATCH 1/4 #4] Add emergency property in TelephonyCall.

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

Let's use r=hsinyi as what we've been doing ;)

Comment 33

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 747755 [details] [diff] [review]
[PATCH 3/4 #3] Update emergency property for outgoing call.

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

Please address the comment and rebase this patch since Bug 869306 made some changes.

::: dom/telephony/Telephony.cpp
@@ +456,4 @@
>    }
>  
>    nsRefPtr<TelephonyCall> call =
> +    TelephonyCall::Create(this, aNumber, aCallState, aCallIndex, aIsEmergency);

Need to update |TelephonyCall::Create()| in |Telephony::CallStateChanged()| as well.

::: dom/telephony/TelephonyCall.h
@@ +39,4 @@
>  
>    static already_AddRefed<TelephonyCall>
>    Create(Telephony* aTelephony, const nsAString& aNumber, uint16_t aCallState,
> +         uint32_t aCallIndex = kOutgoingPlaceholderCallIndex, bool aEmergency = false);

nit: 80 chars per line

Comment 34

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 747753 [details] [diff] [review]
[PATCH 1/4 #4] Add emergency property in TelephonyCall.

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

Let's use r=hsinyi as what we've been doing ;)

Please rebase this patch since Bug 869306 made some changes.

Comment 35

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 747754 [details] [diff] [review]
[PATCH 2/4 #3] Pass emergency info from ril to dom.

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

Please rebase this patch since Bug 869306 made some changes.

Comment 36

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

Attachment: [PATCH 3/4 #4] Update emergency property for outgoing call.

Comment 37

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 748677 [details] [diff] [review]
[PATCH 3/4 #4] Update emergency property for outgoing call.

Perfect, thanks!

Hi Gina,
Minor change to BT code. Would you help review it again? :)

Comment 38

[Gina Yeh [:gyeh] [:ginayeh]]

Comment on attachment 748677 [details] [diff] [review]
[PATCH 3/4 #4] Update emergency property for outgoing call.

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

Looks great. Thanks!

Comment 39

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

https://tbpl.mozilla.org/?tree=Try&rev=ff9694ec7938

Comment 40

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

Attachment: Part 1: Add emergency property in TelephonyCall. r=htsai. sr=mounir.

Comment 41

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

Attachment: (x) Part 2: Pass emergency info from ril to dom. r=htsai.

Comment 42

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

Attachment: Part 3: Update emergency property for outgoing call. r=htsai, gyeh.

Comment 43

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

Attachment: (x) Part 4: Add test for emergency label. r=htsai.

Comment 44

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

Comment on attachment 748700 [details] [diff] [review]
(x) Part 2: Pass emergency info from ril to dom. r=htsai.

some code modification for updating 'emergency' flag in ril_worker.js

Comment 45

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

Attachment: Part 2: Pass emergency info from ril to dom. r=htsai.

Comment 46

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Comment on attachment 748717 [details] [diff] [review]
Part 2: Pass emergency info from ril to dom. r=htsai.

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

::: dom/system/gonk/ril_worker.js
@@ +3266,5 @@
>          }
> +
> +        // Set flag for outgoing emergency call.
> +        if (newCall.isOutgoing && this._isEmergencyNumber(newCall.number)) {
> +          newCall.isEmergency = true;

nit: we can have the following:

newCall.isEmergency = newCall.isOutgoing && this._isEmergencyNumber(newCall.number);

Comment 47

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Aknow, if your patches are ready to push, simply set checkin? or add checkin-needed keyword. I (and other devs with privilege) will be happy to help push them. :)

Comment 48

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

Comment on attachment 748717 [details] [diff] [review]
Part 2: Pass emergency info from ril to dom. r=htsai.

some code modification for updating 'emergency' flag in ril_worker.js

Comment 49

[Hsin-Yi Tsai (she/her) [:hsinyi]]

http://hg.mozilla.org/projects/birch/rev/7189ea1fb7ba
http://hg.mozilla.org/projects/birch/rev/cf7a3a466ffe
http://hg.mozilla.org/projects/birch/rev/0fddb0b4eb1f
http://hg.mozilla.org/projects/birch/rev/676b4a86c55b

Comment 50

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Attachment: Part 4: Add test for emergency label. r=htsai.

Modified manifest.ini

Comment 51

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/7189ea1fb7ba
https://hg.mozilla.org/mozilla-central/rev/cf7a3a466ffe
https://hg.mozilla.org/mozilla-central/rev/0fddb0b4eb1f
https://hg.mozilla.org/mozilla-central/rev/676b4a86c55b

Comment 52

[Anshul]

Should this also be uplifted to v1.0.1 as it blocks a certification blocker bug for 1.0.1?

Comment 53

[Jonas Sicking (:sicking) No longer reading bugmail consistently]

It's currently not listed as blocking any tef+ bugs. If you think something that's not currently listed as tef+ should be a blocker then please nominate that. If this bug blocks something that is already tef+ then please add the dependency.

Comment 54

[Beatriz Rodríguez [:brg]]

It is not blocking certification, it was reported during certification but not as a blocker. We should add it in v1.1.

Comment 55

[Hsin-Yi Tsai (she/her) [:hsinyi]]

leo? since it blocks leo+ bug 870187

Comment 56

[John Hammink]

On B02 (4/26 build) in certification lab in Chile:

Once the calls are initiated, we expect to see a label stating that it's an emergency number that's just been dialed.

However, in the case of 112, it says only "Conectando.." (in the case of Chile).  This needs to be rechecked in Columbia with "123"

On 5/13 build: 
112 is working correctly (at least in Chile).
123 needs to be rechecked in Columbia.

Comment 57

[David Palomino [:dpalomino]]

Thanks John for the update!

Current behaviour is acceptable for 1.0.1. 

Thks!
David

Comment 58

[Hsin-Yi Tsai (she/her) [:hsinyi]]

Hi Anshul,

Any update of Commercial RIL? Is this ready to be landed to b2g18? Thanks.

Comment 59

[Anshul]

Hsin-Yi, yes we have the support since few days, sorry for the delay in responding to your query.

Comment 60

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Anshul from comment #59)
> Hsin-Yi, yes we have the support since few days, sorry for the delay in
> responding to your query.

Thanks, Anshul.
I'll be clearing [NO_UPLIFT] whiteboard.

Hi Aknow,
Let's land this into mozilla-b2g18. Please provide your b2g18-patches, and then set checkin? for each attachment. Thank you.

Comment 61

[Ryan VanderMeulen [:RyanVM]]

You don't need to set checkin? on each patch. This bug comes up in the regular leo uplift queries, so I just need the b2g18 patches and we're good here.

Comment 62

[Hsin-Yi Tsai (she/her) [:hsinyi]]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #61)
> You don't need to set checkin? on each patch. This bug comes up in the
> regular leo uplift queries, so I just need the b2g18 patches and we're good
> here.

Cool, it's simpler than what I am aware of. Thank you for clarifying. :)

Comment 63

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

Attachment: [b2g18] Part 1: Add emergency property in TelephonyCall.

Comment 64

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

Attachment: [b2g18] Part 2: Pass emergency info from ril to dom.

Comment 65

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

Attachment: [b2g18] Part 3: Update emergency property for outgoing call.

Comment 66

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

Attachment: [b2g18] Part 4: Add test for emergency label.

Comment 67

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

Attachment: [b2g18] Part 1: Add emergency property in TelephonyCall.

Comment 68

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

Attachment: [b2g18] Part 2: Pass emergency info from ril to dom.

Comment 69

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

Attachment: [b2g18] Part 3: Update emergency property for outgoing call.

Comment 70

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

Attachment: [b2g18] Part 4: Add test for emergency label.

Comment 71

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

b2g18-patches are ready and attached.

Comment 72

[Ryan VanderMeulen [:RyanVM]]

I attempted to land this on b2g18, but had to backout due to Marionette failures.
https://tbpl.mozilla.org/php/getParsedLog.php?id=23637395&tree=Mozilla-B2g18

Comment 73

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

Hi Ryan,

Sorry for the test fail.
The failed test case is what I add for this bug.
It works well when I test on:
  b2g emultaor (w/ external/qemu on b2g/master head) + gecko (mozilla-b2g18)
But it indeed fail when I push the change to try server.
I'm now wondering whether it is cause by the different version of emulator.

Hi Jonathan

Is the version of emulator used on mozilla-center try the same as which on mozilla-b2g18 try?
How could I know the exact version of emulator (external/qemu) used on those try server?

Comment 74

[Jonathan Griffin (:jgriffin)]

We have different versions of the emulator on m-c and mozilla-b2g18; the mozilla-b2g18 one is older.  If you need a new emulator on that branch, we can file a bug to get one uploaded.  I'll take care of that.

Comment 75

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

Attachment: [b2g18] Part 1#2: Add emergency property in TelephonyCall.

Comment 76

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

Attachment: [b2g18] Part 4#2: Add test for emergency label.

Comment 77

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

Hi Ryan,

Problem confirmed. It's because of the version of emulator.
Even after the update, it is still not new enough for the test case.
We decide not to check-in the test case so please help me land the patch from part 1 to part 3.
It works well on the try. 

https://tbpl.mozilla.org/?tree=Try&rev=ec770d408ec5

Comment 78

[Ryan VanderMeulen [:RyanVM]]

(In reply to Szu-Yu Chen [:aknow] from comment #77)
> We decide not to check-in the test case

As confidence-inspiring as that is... :)

https://hg.mozilla.org/releases/mozilla-b2g18/rev/5a45514475ef
https://hg.mozilla.org/releases/mozilla-b2g18/rev/2a7227e64cfb
https://hg.mozilla.org/releases/mozilla-b2g18/rev/87bec4ec4aff

Comment 79

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/5a45514475ef
https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/2a7227e64cfb
https://hg.mozilla.org/releases/mozilla-b2g18_v1_1_0_hd/rev/87bec4ec4aff

Comment 80

[FangChen]

the issues has been reopen in FFOS 1.1,The wording lable must be changed to "Llamada de emergencia" in Spanish,when an emergency call is established

built ID:20130820074633
QC RIL version:
"ro.build.firmware_revision=V1.01.00.01.019.180"
gaia commit:
4916f82 Merge branch 'zte/ics_strawberry_cdr' of ssh://10.67.16.41:29418/quic/lf/releases/gaia into ics_strawberry_cdr
gecko commit:
a1c2bb0 ZRL modify ACCEPT Http head for MMS

Comment 81

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

(In reply to FangChen from comment #80)
> the issues has been reopen in FFOS 1.1,The wording lable must be changed to
> "Llamada de emergencia" in Spanish,when an emergency call is established
> 
> built ID:20130820074633
> QC RIL version:
> "ro.build.firmware_revision=V1.01.00.01.019.180"
> gaia commit:
> 4916f82 Merge branch 'zte/ics_strawberry_cdr' of
> ssh://10.67.16.41:29418/quic/lf/releases/gaia into ics_strawberry_cdr
> gecko commit:
> a1c2bb0 ZRL modify ACCEPT Http head for MMS

Should be Bug 870187.
This is the bug for gaia UI.