1067859 - With an established call and calling to second phone number the sound alert for new SMS does not beep

Status: VERIFIED FIXED

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

Description

[Loli (:lolimartinezcr)]

Flame
2.1
Gecko: 3953700
Gaia: 713448b

Reproducible: 100%

Pre-requisites:
A call established.

STRs:
1. Tap "add phone number" button.
2. Tap dialer button.
3. Write phone number.
4. Tap Call button.
5. While call is connecting, device receives a new message.

Actual result:
 the sound alert for new SMS does not beep.

And once user has done this STRs, when second call is finished (not established) and only a call is established, SMS received does not beep.

Expected result:
 the sound alert for new SMS does beep

Comment 1

[Maria Angeles Oteo (:oteo)]

David, can you have a look at it? Thanks a lot

Comment 2

[David Garcia [:davidg][retired]]

Sure!

Working on it

Comment 3

[David Garcia [:davidg][retired]]

Attachment: patch

The problem was that I was only playing the notification in telephony channel when the call was in 'connected' state. When you are calling another number the call is in 'alerting' state, and the notification should be played through telephony channel in that case also.

Comment 4

[Etienne Segonzac (:etienne)]

Comment on attachment 8490655 [details] [review]
patch

looks good!

But I'd like to double check with Hsin-Yi if we should maybe add other states or consider that all states should make us bump the notification sound channel.

Comment 5

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

(In reply to Etienne Segonzac (:etienne) from comment #4)
> Comment on attachment 8490655 [details] [review]
> patch
> 
> looks good!
> 
> But I'd like to double check with Hsin-Yi if we should maybe add other
> states or consider that all states should make us bump the notification
> sound channel.

Not sure if I understand the issue correctly ... :P But it looks to me that it might be more proper to use "telephony.active" as a condition as |if (telephony.active)| means telephony service is using audio channel.

Comment 6

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

(In reply to Hsin-Yi Tsai [:hsinyi] from comment #5)
> (In reply to Etienne Segonzac (:etienne) from comment #4)
> > Comment on attachment 8490655 [details] [review]
> > patch
> > 
> > looks good!
> > 
> > But I'd like to double check with Hsin-Yi if we should maybe add other
> > states or consider that all states should make us bump the notification
> > sound channel.
> 
> Not sure if I understand the issue correctly ... :P But it looks to me that
> it might be more proper to use "telephony.active" as a condition as |if
> (telephony.active)| means telephony service is using audio channel.

Besides, telephony.active covers the case of conference call.

Comment 7

[David Garcia [:davidg][retired]]

Comment on attachment 8490655 [details] [review]
patch

I think the Hsin-Yi comment totally makes sense. So, I changed it to use telephony.active. 
Do you think this way is better Etienne?

Comment 8

[Etienne Segonzac (:etienne)]

Comment on attachment 8490655 [details] [review]
patch

Awesome, much better :)

Thanks Hsin-Yi!

Comment 9

[David Garcia [:davidg][retired]]

Master: ff71aa473bfa7df5667ddf976ce6b81f3346ef1c

Comment 10

[Maria Angeles Oteo (:oteo)]

Loli can you verify the feature again? In case all is fine, we could try the approval to 2.1 of this patch so all the feature is completed in that branch. Thanks!

Comment 11

[Loli (:lolimartinezcr)]

(In reply to Maria Angeles Oteo (:oteo) from comment #10)
> Loli can you verify the feature again? In case all is fine, we could try the
> approval to 2.1 of this patch so all the feature is completed in that
> branch. Thanks!

Tested and working
2.2 
Gecko-93e8d33
Gaia-dbc8904

Pending 2.1

Comment 12

[David Garcia [:davidg][retired]]

Comment on attachment 8490655 [details] [review]
patch

[Approval Request Comment]
[Bug caused by] (feature/regressing bug #): 976678
[User impact] if declined: User will not hear alerts while trying to call (call in alerting state) another number.
[Testing completed]: yes
[Risk to taking this patch] (and alternatives if risky): 
[String changes made]: no

Comment 13

[[:fabrice] Fabrice Desré]

Comment on attachment 8490655 [details] [review]
patch

relatively uncommon use case, and no data loss.

Comment 14

[Duane Dixon [:ddixon]]

Issue is verified fixed in Flame 2.2

Actual Results: Phone vibrates and audibly beeps out of earpiece when message is received while making a second call. 

Device: Flame Master
Build ID: 20141014040203
Gaia: 4f86c631e0465c0e56ccebeb1324fd28be9ea32f
Gecko: 54217864bae9
Version: 36.0a1 (Master)
Firmware Version: v180
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Leaving 'verifyme' tag for 2.1 patch when it has landed.

Comment 15

[John Thomas [:Johnt]]

This issue is still occurring on the 2.2 KK Full Flash (319mb), Flame 2.1 KK Shallow Flash (319mb) and the Flame 2.0 KK Shallow Flash (319mb)

A new bug has be written on this issue. Please see https://bugzilla.mozilla.org/show_bug.cgi?id=1105083

Result: The sound alert for new SMS does not occur.

Flame 2.2

Device: Flame 2.2 (319mb)(Kitkat Base)(Shallow Flash)
Build ID: 20141125040209
Gaia: 824a61cccec4c69be9a86ad5cb629a1f61fa142f
Gecko: acde07cb4e4d
Version: 36.0a1 (2.2)
Firmware Version: v188-1
User Agent: Mozilla/5.0 (Mobile; rv:36.0) Gecko/36.0 Firefox/36.0

Flame 2.1

Device: Flame 2.1 (319mb)(Kitkat Base)(Shallow Flash)
BuildID: 20141125001201
Gaia: 1bdd49770e2cb7a7321e6202c9bf036ab5d8f200
Gecko: db893274d9a6
Version: 34.0 (2.1)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:34.0) Gecko/34.0 Firefox/34.0


Flame 2.0

Device: Flame 2.0 (319mb)(Kitkat Base)(Shallow Flash)
BuildID: 20141121000203
Gaia: 626d4d11a8c7e55022c1f364abb72ea340e2f1e7
Gecko: 74a294e72d0a
Version: 32.0 (2.0)
Firmware: V188-1
User Agent: Mozilla/5.0 (Mobile; rv:32.0) Gecko/32.0 Firefox/32.0

Comment 16

[KTucker [:KTucker][Inactive 3/4/2016]]

This must have regressed on master since the last verification. Since a new issue has been written let's leave this issue closed.

Comment 17

[Shally Li（leave from Mozilla）]

Per Comment 15 & Comment 16,clear "verifyme".