976678 - [zffos1.3][P3](Local) During voice calls the sound alert for new SMS does not beep.

Status: VERIFIED FIXED

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

Description

[FangChen]

User Agent: Mozilla/5.0 (Windows NT 6.1; Trident/7.0; SLCC2; .NET CLR 2.0.50727; .NET CLR 3.5.30729; .NET CLR 3.0.30729; Media Center PC 6.0; .NET4.0C; .NET4.0E; TCO_20140226031831; rv:11.0) like Gecko

Steps to reproduce:

 A.- Overview Description (technical background, concise explanation of the bug):
    During voice calls the sound alert for new SMS does not beep.
    
    ________________________________________________________________________________
    B.- Steps to Reproduce (initial conditions, required resources, step by step instructions to reproduce):
    1- Make a voice call with the DuT to any valid number and keep it active.
2- Send a SMS to the DuT.
3- Verify if the alert sound is reproduced when the short message arrives during the voice call.



Actual results:

The new short message is not alerted with an alert sound.



Expected results:

The new message should be indicated with a short "beep" sound to make the user look at the screen.

Comment 1

[FangChen]

Built ID :20140218180032
OS version
1.3.0.0-prerelease
QC RIL commit:
a406d321  Add null check when handling set preferred network type
QC RIL TAG version: AU_LINUX_GECKO_B2G_JB_3.2.01.03.00.112.223

gaia commit:
744fb691 Merge pull request #15526 from steveck-chung/bug-961231

gecko commit:
6840e8c2 Bumping gaia.json for 1 gaia-1_3 revision(s) a=gaia-bump

Comment 2

[Beatriz Rodríguez [:brg]]

The decission here is to vibrate when a notification is received. IMHO it is enough to make the user know that there is something new.

Comment 3

[Juan Perez-Bedmar [:juanpbf]]

(In reply to Beatriz Rodríguez [:brg] from comment #2)
> The decission here is to vibrate when a notification is received. IMHO it is
> enough to make the user know that there is something new.

Honestly, I think that the handset vibrating in your ear when a SMS arrives is quite enoying. As far as I know, the usual implementation is a small beep, and I really think we should do the same in Firefox OS. 

I agree that this is not a blocking/critical issue, as the user is actually notified of the incoming SMS, but, IMHO, this should change for the future --> nominating to 1.5?

Comment 4

[howie [:howie]]

Put to component SMS first because still not confirm the behavior.

Comment 5

[Julien Wajsberg [:julienw]]

In the SMS app we call navigator.vibrate and navigator, and a Audio object in the 'notification' audio channel.

Just tell us what we should change and we can do it but the bug is currently not actionable for us.

Comment 6

[Beatriz Rodríguez [:brg]]

Ni to UX to get their feedback?

Comment 7

[Joe Cheng [:jcheng] (please needinfo)]

add this to the targeted list

Comment 9

[Stephany Wilkes]

Sorry - I don't understand the flag, as it appears the behavior was specified earlier. Please specify what is still needed here.

Comment 10

[Beatriz Rodríguez [:brg]]

(In reply to Stephany Wilkes from comment #9)
> Sorry - I don't understand the flag, as it appears the behavior was
> specified earlier. Please specify what is still needed here.
Are you ok with changing the current behaviour: vibrate with new sms(or any other notification) during a call to the new one requested: beep with new sms(or any other notification) during a call?

Sorry if the behaviour was specified earlier, I want to have it clear.

Comment 11

[Julien Wajsberg [:julienw]]

Note that the "vibrate" can be actually enabled/disabled in settings. So you'd like to override the setting when in a call, is that it? Would you that to be configurable?

Comment 12

[Stephany Wilkes]

Got it. This one belongs to Carrie, who owns Comms UX. Thanks for the clarification!

Comment 13

[Carrie Wang [New account: [e-mail]]]

I've discussed this with Messaging owner and we'd suggest following the alert/vibration settings. If both the alert and vibration are switched on, then the user can hear the beep and the vibration during the call. If both of the settings are off, then he/she will not received neither sound or vibration. We think the behavior will be consistent in this way. Thanks!

Comment 14

[Julien Wajsberg [:julienw]]

Carrie, does that mean we should close wontfix ?

Comment 15

[Noemí Freire (:noemi)]

Hi,

Please see below the current behavior with today's (4/30) master build:
Device: Hamachi
BuildId: 20140430075058
Gecko: 13b2542
Gaia: 08ea844
Platform version: 32.0a1

Incoming SMS while active call:
- vibrate ON + alert ON -> vibration (KO no alert sound)
- vibrate ON + alert OFF -> vibration (OK)
- vibrate OFF +  alert ON -> neither alert sound nor vibration (KO no alert sound)
- vibrate OFF + alert OFF -> neither alert sound nor vibration (OK)

Incoming SMS w/o active call:
- vibrate ON + alert ON -> vibration + alert sound (OK)
- vibrate ON + alert OFF -> vibration (OK)
- vibrate OFF +  alert ON -> alert sound (OK)
- vibrate OFF + alert OFF -> neither alert sound nor vibration (OK)

so, in case of having an active call it is not currently working as UX pointed out in comment 13, it means, the configuration set by users is not working as it should in case of alert sound.

Comment 16

[Carrie Wang [New account: [e-mail]]]

Hi,I think the best way to prevent the confusion of the user expectation is following what users set for alert. In this way, the behavior will be consistent. Hence, I stick with what I suggest in comment 13. Thanks!

Comment 17

[Noemí Freire (:noemi)]

(In reply to Carrie Wang [:carrie] from comment #16)
> Hi,I think the best way to prevent the confusion of the user expectation is
> following what users set for alert. In this way, the behavior will be
> consistent. Hence, I stick with what I suggest in comment 13. Thanks!

OK, so the scenarios pending to be fixed would be:
Incoming SMS while active call:
- vibrate ON + alert ON -> vibration (KO no alert sound)
- vibrate OFF +  alert ON -> neither alert sound nor vibration (KO no alert sound)

Comment 18

[Julien Wajsberg [:julienw]]

The code that does the notification is [1].

[1] https://github.com/mozilla-b2g/gaia/blob/98ca8c55dbe2f21a8661d0eaa87f34d316c3bc98/apps/sms/js/notify.js#L33-L43

I can see some possibilities:

1. we create 2 Audio objects, one for the channel "notification", one for the channel "telephony". As far as I understood, the channel "telephony" is muted when we're not in a call, and the channel "notification" is muted when we're in a call. That would also allow us to have 2 different sounds for these notifications.

2. we have a new audio channel "sms-notification", or something, that would really go to one channel or the other.

3. we can specify 2 channels in mozAudioChannel, for example separated with a comma.


I think 1. is what we want, because we want to specify a different sound.

Baku, do you think 1) would work fine?

Comment 19

[Maria Angeles Oteo (:oteo)]

This bug is a certification waiver and it's important that could be fixed in next release, setting ni to Marcos Chen too so he can answer Julien's question

Comment 20

[Marco Chen [:mchen]]

Hi FengChen,

May I know whether the behavior now is the same with V1.3/V1.4 or not?
If yes, then it is not a bug but is a new feature.

Hi Julien,

Basically I agreed with your suggestion 1 and I had proposed another solution on [1].
That will be more general and cover more apps.

The problem now is [2] introduced some workaround on V2.0 to fix competing problem between two telephony channels from two apps. It means your SMS app can't use telephony channel anymore during voice call or SMS app will force callscreen app to put it's call on hold.

Or SMS can still use notification channel during voice call but SMS needs to be on the foreground because only foreground app can fire sound always.

Hi Ivan,

Could media functional team take care this kind of issues?
As I know there is a plan to re-construct audio competing policy.

Thanks.

[1] Bug 871467 - [AudioChannel] To generate alert tone during in_call state when alarm/notification is fired.
[2] Bug 1016277 - If two different apps try to use the 'telephony' channel at the same time both apps can play audio.

Comment 21

[Julien Wajsberg [:julienw]]

I think I missed something in my previous comment.

When receiving a message, there are 2 cases:

1. the SMS app is foreground, the thread receiving the message is open
=> we don't display a notification and we use the code in comment 18

2. the SMS app is either background or open in another panel
=> we display a notification, and the System app is responsible for vibrating + playing audio.

Comment 22

[Marco Chen [:mchen]]

(In reply to Julien Wajsberg [:julienw] from comment #21)
> I think I missed something in my previous comment.
> 2. the SMS app is either background or open in another panel
> => we display a notification, and the System app is responsible for
> vibrating + playing audio.

As I mentioned that according to [2], system app (always in the background) can only use telephony channel to fire beep but this will force callscreen app to put current voice call into on hold state.
Because there is only one process can grant to use telephony channel and the policy is LIFO.

[2] Bug 1016277 - If two different apps try to use the 'telephony' channel at the same time both apps can play audio.

Comment 23

[Julien Wajsberg [:julienw]]

Yep, I was not saying you were wrong here :) I only wanted to precise what I was saying.

Comment 24

[Beatriz Rodríguez [:brg]]

(In reply to Marco Chen [:mchen] from comment #20)
> Hi FengChen,
> 
> May I know whether the behavior now is the same with V1.3/V1.4 or not?
> If yes, then it is not a bug but is a new feature.

I do not know if Fangchen will be able to answer this question, but I would like to clarify that testing:
Flame 2.0(Gecko-cee7379.Gaia-aa4f795) and master(Gecko-03ca84f.Gaia-e377ad3) the behaviour is the same described in comment 17.

Comment 25

[Maria Angeles Oteo (:oteo)]

(In reply to Beatriz Rodríguez [:brg] from comment #24)
> (In reply to Marco Chen [:mchen] from comment #20)
> > Hi FengChen,
> > 
> > May I know whether the behavior now is the same with V1.3/V1.4 or not?
> > If yes, then it is not a bug but is a new feature.
> 
I have checked in v1.3 (Gecko-0618e14.Gaia-23f55be) and the behavior is the same as described in comment 17 and as Beatriz has confirmed the same than in master and v2.0.

It's not a regression but I would not say this is a new feature, I'd said that this a bug that we have in FirefoxOS from the beginning and we have not fixed yet and for sure we need to fix to complete the functionality and solve this certification waiver.

Comment 26

[FangChen]

We have no product to support 1.4,so I can not make sure if the behavior is the same

(In reply to Marco Chen [:mchen] from comment #20)
> Hi FengChen,

May I know whether the behavior now is the same with V1.3/V1.4
> or not?
If yes, then it is not a bug but is a new feature.

Hi Julien,
> Basically I agreed with your suggestion 1 and I had proposed another
> solution on [1].
That will be more general and cover more apps.

The problem
> now is [2] introduced some workaround on V2.0 to fix competing problem
> between two telephony channels from two apps. It means your SMS app can't
> use telephony channel anymore during voice call or SMS app will force
> callscreen app to put it's call on hold.

Or SMS can still use notification
> channel during voice call but SMS needs to be on the foreground because only
> foreground app can fire sound always.

Hi Ivan,

Could media functional team
> take care this kind of issues?
As I know there is a plan to re-construct
> audio competing policy.

Thanks.

[1] Bug 871467 - [AudioChannel] To
> generate alert tone during in_call state when alarm/notification is fired.
> [2] Bug 1016277 - If two different apps try to use the 'telephony' channel
> at the same time both apps can play audio.

Comment 27

[David Garcia [:davidg][retired]]

Attachment: davidg patch proposal

I have been looking at this bug, and I'm proposing this patch, that I tried on master code and seems to work fine. I still need to add some tests, but I wanted to check with you first, just in case I'm doing something stupid before starting to work on the tests.
Basically, in system I'm checking if we are in a call, and switch to telephony channel to play the notification in that case.
I'm ni Marco, because he said system could not be able to play telephony channel, but it seems to work for me, so probably I'm missing something.

Comment 28

[Julien Wajsberg [:julienw]]

Comment on attachment 8460906 [details] [review]
davidg patch proposal

Redirecting to Étienne who might redirect to someone more involved with this.

Comment 29

[Marco Chen [:mchen]]

(In reply to David Garcia [:davidg] from comment #27)
> I'm ni Marco, because he said system could not be able to play telephony
> channel, but it seems to work for me, so probably I'm missing something.

Hi David,

If your patch can work correctly without putting voice call on hold then it probably means that call screen app is running on the same process with sytem app. So it didn't compete with call screen app.

But please test that 
  1. Accept a incoming call.
  2. Accept a VOIP/Loop call. (now voice call is on hold)
  3. Recieve a SMS message. (You will hear notification from system app via telephony channel)

Then you will find VOIP call is suspend and voice call is back because telephony channel from notification sound competes VOIP/Loop app and it wins this game. I think this is a problem.

Comment 30

[Ivan Tsay (:ITsay)]

(In reply to Marco Chen [:mchen] from comment #20)
> ...
> 
> Hi Ivan,
> 
> Could media functional team take care this kind of issues?
> As I know there is a plan to re-construct audio competing policy.
> 
> Thanks.
> ...

Actually there is the long term plan to move the audio competing management from gecko to gaia system app without changing the existing audio competing rules. As you mentioned this is the issue on audio competing rules and it seems to me the rules have to be reviewed and probably revised. The question here is differently from our original plan on re-architect audio completing management. Will talk to engineer and see how to improve this phase by phase.

Comment 31

[David Garcia [:davidg][retired]]

Comment on attachment 8460906 [details] [review]
davidg patch proposal

I checked the problem that Marco Chen pointed and he was right. 
My solution was to instead of using soundManager to check if there is a call ongoing, I check if there is any call in connected state.
That way, in the scenario Marcho Chen pointed, the call is on hold, so the telephony channel will not be used, so VoIP/Loop doesn't lose the channel.
I also reduced the volume when playing over the telephony channel, because it was so loud that it was a bit annoying. 
I also added unit tests.

Comment 32

[Etienne Segonzac (:etienne)]

Comment on attachment 8460906 [details] [review]
davidg patch proposal

The patch definitely looks good!
I'd just like to ask Jaoo to confirm that this won't cause weird competing issue, or even put the call on hold...

Comment 33

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Marco Chen [:mchen] from comment #29)

> But please test that 
>   1. Accept a incoming call.
>   2. Accept a VOIP/Loop call. (now voice call is on hold)
>   3. Recieve a SMS message. (You will hear notification from system app via
> telephony channel)

In the scenario you described above as the priority of telephony audio channels is higher than the notification audio channel the SMS notification is not heard.

(In reply to David Garcia [:davidg] from comment #31)
> Comment on attachment 8460906 [details] [review]
> davidg patch proposal

> That way, in the scenario Marcho Chen pointed, the call is on hold, so the
> telephony channel will not be used, so VoIP/Loop doesn't lose the channel.

The SMS notification is not heard (If I'm not wrong and as I commented earlier) due the audio competing policy implemented by the audio channel service but I guess this is not an issue.

(In reply to Etienne Segonzac (:etienne) (PTO -> August 12) from comment #32)
> Comment on attachment 8460906 [details] [review]
> davidg patch proposal

> I'd just like to ask Jaoo to confirm that this won't cause weird competing
> issue, or even put the call on hold...

Taking the patch the SMS notification is heard when the GSM call is connected which was the issue in this bug. Moreover no audio competing issues between the callscreen and Loop app.

Comment 34

[David Garcia [:davidg][retired]]

Comment on attachment 8460906 [details] [review]
davidg patch proposal

Good to merge then?

Comment 35

[Etienne Segonzac (:etienne)]

Comment on attachment 8460906 [details] [review]
davidg patch proposal

Needs rebasing but otherwise good to go with a green try!

Comment 36

[Marco Chen [:mchen]]

(In reply to José Antonio Olivera Ortega [:jaoo] PTO till 8/18 from comment #33)
> (In reply to David Garcia [:davidg] from comment #31)
> > Comment on attachment 8460906 [details] [review]
> > davidg patch proposal
> 
> > That way, in the scenario Marcho Chen pointed, the call is on hold, so the
> > telephony channel will not be used, so VoIP/Loop doesn't lose the channel.
> 
> The SMS notification is not heard (If I'm not wrong and as I commented
> earlier) due the audio competing policy implemented by the audio channel
> service but I guess this is not an issue.
 
I think this is a problem. The general issue on this bug should be "the SMS notification sound can't be heard by user during any telephony channel is using. ex: GSM call or Loop/VOIP call".

If user think that he/her want to be notified by SMS notification sound during GSM call why don't user expect to be notified too during Loop/VOIP call.

Or if you want to split it to two different cases then maybe the follow up bug should be fired.

Comment 37

[Maria Angeles Oteo (:oteo)]

Bug 1057988 created to implement it for Loop calls

Comment 38

[José Antonio Olivera Ortega [:jaoo]]

(In reply to Marco Chen [:mchen] from comment #36)

> > The SMS notification is not heard (If I'm not wrong and as I commented
> > earlier) due the audio competing policy implemented by the audio channel
> > service but I guess this is not an issue.
>  
> I think this is a problem. The general issue on this bug should be "the SMS
> notification sound can't be heard by user during any telephony channel is
> using. ex: GSM call or Loop/VOIP call".
> 
> If user think that he/her want to be notified by SMS notification sound
> during GSM call why don't user expect to be notified too during Loop/VOIP
> call.

As the telephony audio channel priority is higher that the notification audio channel the alert cannot be heard (if I'm not wrong). Unless the policy changes the issue will persists while in a Loop call. As I don't think the policy is going to change to fix this issue we will need to figure out a workaround for Loop. Something like David proposed in this patch might work, but we would need to know whether the user is on a Loop call by the time the SMS is received.

Comment 39

[David Garcia [:davidg][retired]]

Master: a25ae14dbd2fe3e25144a7064efc0cc4e31042b8

Comment 40

[Loli (:lolimartinezcr)]

Tested and new bug:
Flame
2.1
Gecko-f816f7e
Gaia-c7b55ed

https://bugzilla.mozilla.org/show_bug.cgi?id=1064807

Comment 41

[Julien Wajsberg [:julienw]]

Next time, please squash the commits...

Comment 42

[Loli (:lolimartinezcr)]

New bug detected:
https://bugzilla.mozilla.org/show_bug.cgi?id=1067859

Comment 44

[Sue]

Attachment: verify_video.MP4

This issue has been verified successfully on Flame v2.1
STR:
1. Make a voice call with the DuT to any valid number and keep it active.
2. Send a SMS to the DuT.
3. Verify if the alert sound is reproduced when the short message arrives during the voice call.
**The new message will be indicated with a short "beep" sound and a vibration.
See attachment: verify_video.MP4
Reproducing rate: 0/5
Flame 2.1 versions:
Gaia-Rev        ccb49abe412c978a4045f0c75abff534372716c4
Gecko-Rev       https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22
Build-ID        20141130001203
Version         34.0
Device-Name     flame
FW-Release      4.4.2
FW-Incremental  eng.cltbld.20141130.034738
FW-Date         Sun Nov 30 03:47:49 EST 2014
Bootloader      L1TC00011880

Comment 45

[Sue]

According to comment 44, change the Status to VERIFIED