Closed Bug 1067859 Opened 5 years ago Closed 5 years ago

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

Categories

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

ARM
Gonk (Firefox OS)
defect
Not set

Tracking

(b2g-v2.1 affected, b2g-v2.2 verified)

VERIFIED FIXED
Tracking Status
b2g-v2.1 --- affected
b2g-v2.2 --- verified

People

(Reporter: lolimartinezcr, Assigned: davidg)

References

Details

Attachments

(1 file)

46 bytes, text/x-github-pull-request
etienne
: review+
Details | Review
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
See Also: → 976678
David, can you have a look at it? Thanks a lot
Flags: needinfo?(david.garciaparedes)
Sure!

Working on it
Assignee: nobody → david.garciaparedes
Flags: needinfo?(david.garciaparedes)
Attached file 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.
Attachment #8490655 - Flags: review?(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.
Attachment #8490655 - Flags: review?(etienne) → review+
Flags: needinfo?(htsai)
(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.
Flags: needinfo?(htsai)
(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 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?
Attachment #8490655 - Flags: review+ → review?(etienne)
Comment on attachment 8490655 [details] [review]
patch

Awesome, much better :)

Thanks Hsin-Yi!
Attachment #8490655 - Flags: review?(etienne) → review+
Master: ff71aa473bfa7df5667ddf976ce6b81f3346ef1c
Status: NEW → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
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!
Flags: needinfo?(lolimartinezcr)
Keywords: verifyme
(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
Flags: needinfo?(lolimartinezcr)
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
Attachment #8490655 - Flags: approval-gaia-v2.1?
Comment on attachment 8490655 [details] [review]
patch

relatively uncommon use case, and no data loss.
Attachment #8490655 - Flags: approval-gaia-v2.1? → approval-gaia-v2.1-
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.
Status: RESOLVED → VERIFIED
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(ktucker)
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Depends on: 1105083
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
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage?][failed-verification]
Flags: needinfo?(ktucker)
This must have regressed on master since the last verification. Since a new issue has been written let's leave this issue closed.
QA Whiteboard: [QAnalyst-Triage?][failed-verification] → [QAnalyst-Triage+]
Flags: needinfo?(ktucker)
Per Comment 15 & Comment 16,clear "verifyme".
Keywords: verifyme
You need to log in before you can comment on or make changes to this bug.