866481 - [OPEN_][Dialer]It shows two lines "112" when we make an emergency call in phone lock screen.

Status: VERIFIED FIXED

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

Description

[Firefox_Mozilla]

Attachment: screenshot

Step to reproduce:
1.go into settings-Phone lock to set Passcode lock;
2.lock screen;
3.in unlock screen, tap "emergency call" to make an emergency call(such as 112);
Expected:
1.the screen shows correct;
Actual:
1.it shows two lines which all show 112 in calling.

build ID:20130426165758
build information:
gaia commit:
e25b349 Merge pull request #9273 from gasolin/issue-863126
gecko commit:
6dfd179 Bug 849757 - Part 4: xpcshell tests. r=vicamo, a=tef+
gaia commit:
e25b349 Merge pull request #9273 from gasolin/issue-863126
gecko commit:
6dfd179 Bug 849757 - Part 4: xpcshell tests. r=vicamo, a=tef+
AU:V1.01.00.01.019.085

Comment 1

[José Antonio Olivera Ortega [:jaoo]]

This is also happening on unagi devices.

Comment 2

[Daniel Coloma:dcoloma]

Super-weird behaviour, Etienne, I think there were some changes in the way in which Emergency Calls were made from the lock screen. Could you please have a look?

By the way, this is hapenning when a SIM Card is inserted, when there is no SIM Card the behaviour is even worse (no way to end the call) but we will fill another one for that scenario (bug 866740)

Comment 3

[Kai-Chih Hu [:khu]]

This needs to be fixed before May 21th.

Comment 4

[Etienne Segonzac (:etienne)]

(In reply to Daniel Coloma:dcoloma from comment #2)
> Super-weird behaviour, Etienne, I think there were some changes in the way
> in which Emergency Calls were made from the lock screen. Could you please
> have a look?

The lockscreen emergency call code hasn't moved in months :)
(bug 863595 is not landed yet).

Anyway this bug is bad and easily reproducible, I'm taking a look.

> 
> By the way, this is hapenning when a SIM Card is inserted, when there is no
> SIM Card the behaviour is even worse (no way to end the call) but we will
> fill another one for that scenario (bug 866740)

This is not reproducible on my devices, I wonder if it comes from "below".

Comment 5

[Etienne Segonzac (:etienne)]

So the "emergency call from the lockscreen" feature has been fundamentally broken for a _long_ while (~6 months).

To sum up:
- the lockscreen embeds it's own call screen because there was no way to tell the dialer app to handle this call at the time the feature was developed
- but we have a "telephony-new-call" system message for every call now
- so when dialing an emergency call from the lockscreen we open the lockscreen embedded call screen *and* the real call screen (which is in locked mode)

We need:
- to remove the call screen embedded in the lockscreen
- to handle properly the case where a new outgoing call is started while in locked mode (the only way to do this right now is dialing an emergency call from the lockscreen)

Note that we will lose the striped down design that the embedded call screen had (it will look like a classic, unlocked, outgoing call).

It's not a trivial patch but I'm confident it can be done in time (probably this week).

Comment 6

[Etienne Segonzac (:etienne)]

Hey Zac, wondering how hard it would be to cover this with integration tests?

It's hidden behind the passcode lock so people rarely see this when doing QA.

Comment 7

[Zac C (:zac)]

Etienne, theoretically we can do it but if it automatically starts the call I would not want to do it, for risk that the call is not terminated properly and hangs open. I don't have a device to hand right now.

We have a blocker in sending the right touch events via Marionette so we might not be able to do it immediately.

Comment 9

[Etienne Segonzac (:etienne)]

Attachment: Patch proposal

Simpler than expected :)
Here is a patch, and I'm pretty sure it fixes bug 863595 too!

Comment 11

[Mingming ZHAO]

Attachment: snapshot

Dear Etienne,
I have tried your patch, it did not work.Please refer to the attachment.

Comment 12

[Etienne Segonzac (:etienne)]

(In reply to Mingming ZHAO from comment #11)
> Created attachment 744981 [details]
> snapshot
> 
> Dear Etienne,
> I have tried your patch, it did not work.Please refer to the attachment.

So the good news is, that's progress!
Only 1 call screen left (so this part is fixed), but the call screen looks like an incoming call. (which shouldn't be possible unless the call is "incoming"...)

I'm flashing a phone with the commercial RIL in order to see if I can reproduce this.

Comment 13

[Etienne Segonzac (:etienne)]

(In reply to Mingming ZHAO from comment #11)
> Created attachment 744981 [details]
> snapshot
> 
> Dear Etienne,
> I have tried your patch, it did not work.Please refer to the attachment.

After more testing with the commercial RIL (still couldn't reproduce the issue), a few questions:

* On which branch did you try the patch?
The attached screenshot is weird:
I couldn't find the code paths that display the "Emergency calls only" label on the lockscreen emergency call keypad nor the "Emergency call" label on the call screen (even if you have a contact named "Emergency call" it's not displayed this way).
FYI I tested the patch on gaia master, v1-train and v1.0.1.

* If you had custom patches on your branch (ie patches that weren't upstreamed to the gaia repository) can you try the patch again on a "clean" branch?

* Can you set "ril.debugging.enabled" to true in build/preference.js in order to have RIL logs while reproducing the bug.

* Can you try to reproduce the bug with and without the dialer app already launched to see if the behavior changes (it could).

Thanks, this will help a lot!

Comment 14

[Mingming ZHAO]

Etienne,I am so sorry. I tested the patch again it works well.I am sorry I caused you so much trouble.

The "Emergency calls only" label on the lockscreen emergency call keypad was added by ourself in gaia/apps/system/emergency-call/index.html. The "Emergency call" label on the call screen also was added by ourself. We added an interface "isEmergencyNumber" in gecko to decide whether a number is an emergency call number or not. Then use the interface in gaia to add the "Emergency call" label. I think it will not affect anything.

Comment 16

[Etienne Segonzac (:etienne)]

(In reply to Mingming ZHAO from comment #14)
> Etienne,I am so sorry. I tested the patch again it works well.I am sorry I
> caused you so much trouble.

No problem :)

> 
> The "Emergency calls only" label on the lockscreen emergency call keypad was
> added by ourself in gaia/apps/system/emergency-call/index.html. The
> "Emergency call" label on the call screen also was added by ourself. We
> added an interface "isEmergencyNumber" in gecko to decide whether a number
> is an emergency call number or not. Then use the interface in gaia to add
> the "Emergency call" label. I think it will not affect anything.

Do you know if those changes are going to be upstreamed to the central code repositories?
(this way we can all test with the same code base)

Comment 17

[Mingming ZHAO]

(In reply to Etienne Segonzac (:etienne) from comment #16)
> Do you know if those changes are going to be upstreamed to the central code
> repositories?
> (this way we can all test with the same code base)

Etienne,as far as I know the changes will not been upstreamed.There are some custom behavier.

Comment 18

[Francisco Jordano [:arcturus] [:francisco]]

Hi all,

I've found something weird, again, the special case for countries where is not possible to make emergency calls without sim card.

I'm attaching a video, where you can see how calling '112' sets the lockscreen in a weird status. Also is something that is happening intermittently as you can check on the video.

Comment 19

[Francisco Jordano [:arcturus] [:francisco]]

Attachment: Video of patch applied to 1.0.1 in UK

Comment 20

[Etienne Segonzac (:etienne)]

(In reply to Francisco Jordano [:arcturus] from comment #18)
> Hi all,
> 
> I've found something weird, again, the special case for countries where is
> not possible to make emergency calls without sim card.
> 
> I'm attaching a video, where you can see how calling '112' sets the
> lockscreen in a weird status. Also is something that is happening
> intermittently as you can check on the video.

So...
In a nutshell, only the lockscreen can catch this error right now, and the lockscreen can't tell the callscreen to close itself.

So we can't fix the side effect without bug 823958.

So we can either file a followup bug and land this patch or add this bug to the long list of dependencies on bug 823958.

Comment 21

[Francisco Jordano [:arcturus] [:francisco]]

(In reply to Etienne Segonzac (:etienne) from comment #20)
> So...
> In a nutshell, only the lockscreen can catch this error right now, and the
> lockscreen can't tell the callscreen to close itself.
> 
> So we can't fix the side effect without bug 823958.
> 
> So we can either file a followup bug and land this patch or add this bug to
> the long list of dependencies on bug 823958.

I do agree, since this bug fixes the problem when the we CAN do the call from the lockscreen, we should land it and follow up and make the dependency.

My only concern is Brasil is one of those countries where you cannot make emergency calls without sim inserted.

Cheers!
F.

Comment 22

[Francisco Jordano [:arcturus] [:francisco]]

Comment on attachment 744671 [details] [diff] [review]
Patch proposal

@etienne thanks again for your work, will you open the new bug to follow up in the cases like Brasil and Uk?

Thanks!
F.

Comment 24

[Andrew Overholt [:overholt]]

Since this blocks bug 863595 which is slated for the May 10th milestone, I'm marking this one as such, too.

Comment 25

[Johnny Stenback (:jst)]

Etienne, can this land? If so, please make sure that happens ASAP. Thanks!

Comment 26

[Etienne Segonzac (:etienne)]

https://github.com/mozilla-b2g/gaia/commit/340201b4b7352205dedb00463253de527e2c479a

Comment 27

[John Ford [:jhford] CET/CEST Berlin Time]

Uplifted 340201b4b7352205dedb00463253de527e2c479a to:
v1-train: 6ffd5a675fed78bc27cc4c2bd15af1808477126b
v1.0.1: 2f173e818735b8aa930aa9711d6706a90be2f8c2

Comment 28

[Daniel Coloma:dcoloma]

Hi Etienne,

 This is not working for me with the following config:

 Gecko: 6fb511d
 Gaia: 54dfb62
 RIL: Commercial RIL, AU_LINUX_GECKO_ICS_STRAWBERRY_V1.01.00.01.019.105.
 Device: Unagi

 The effect is still the same described in https://bugzilla.mozilla.org/attachment.cgi?id=744981

 Can qa verify if this already fixed in 1.0.1 (maybe I am doing something wrong)?

Thanks!

Comment 29

[Etienne Segonzac (:etienne)]

(In reply to Daniel Coloma:dcoloma from comment #28)
>  Can qa verify if this already fixed in 1.0.1 (maybe I am doing something
> wrong)?
> 

While waiting for QA, can you take a screenshot from your unagi?

(Every detail matters, and I'm pretty sure at least some details are different since the screenshot linked has some buri specific ui changes.)

Comment 30

[Daniel Coloma:dcoloma]

Tested with today's build with both commercial and opensource RIL. Problem still occurs :(

Gecko-6c835ac
Gaia-e218756

Enclosing screenshots in a while

Comment 31

[Daniel Coloma:dcoloma]

Attachment: OpensourceRIL

Comment 32

[Daniel Coloma:dcoloma]

Attachment: CommercialRIL

Comment 33

[Anshul]

Reopening the issue. Etienne, please have a look.

Comment 34

[Jason Smith [:jsmith]]

Please file a followup bug for the issue you hit and add it as a dependency on this bug.

Comment 35

[Etienne Segonzac (:etienne)]

I'm afraid there was an issue while uplifting the patch.

As you can see the original patch [1] removed the #call-screen element of the |system/emergency-call/index.html|, which makes the last screenshots completely unreproducible.

But, in v1-train it's still here [2]. Which leads me to believe the patch isn't uplifted to v1-train (and probably v1.0.1).

[1] https://github.com/mozilla-b2g/gaia/commit/2b8914b3c70f5c12b9ec42c58d2fcb3bb0e1e8a9#apps/system/emergency-call/index.html

[2] https://github.com/mozilla-b2g/gaia/blob/v1-train/apps/system/emergency-call/index.html#L151-196

Comment 36

[Daniel Coloma:dcoloma]

I am marking then 1.0.1 and 1.1 as affected. Can we please try to uplift asap as this is required for starting next certification round?

Comment 37

[John Ford [:jhford] CET/CEST Berlin Time]

I'm not sure why, but the comment from bug 860570 is in this bug.

v1-train: 6bd8293ad5e9ce2dc7fd4ab07af16b09c5f62639
v1.0.1: cebe8cc32d43a00d5f4aca1d14044a9928c100c2

Comment 38

[bov]

Tested with Inari and Ikura devices.
Build: 20130523002824
COM RIL. 111
Platform version: 18.0 

Steps :
1.Go into Settings, Phone lock and set Passcode lock;
2.Lock screen;
3.Unlock screen, tap "emergency call" to make an emergency call(such as 112);
Expected:
1.The screen shows correctly;
Actual:
1.The screen shows correctly and it is possible to make the emergency call.

Comment 39

[Alissa（leave from Mozilla）]

According to comment 38,delete keywords verifyme.