923544 - [Media][B2G][Settings-Sound] Alert tone is not heard when user recieves an sms

Status: RESOLVED FIXED

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

Description

[Shilpa Suresh]

Attachment: logcat for alert tone

Description:
User tries to change the ringer tone and alert tone in settings app. The changed alert tone does not play when user recieves an SMS from another device.

Repro Steps:
1) Updated Buri v1.2 aurora to Build ID: 20131003004003
2) Go to settings and then tap sounds
3) Under tones, Change the Ringer tone and alerts tone 
4) Recieve an SMS from another device.
5) Observe that the Alerts tone is not heard

Actual:
Alert tone is heard when user recieves an SMS

Expected:
Alert tone plays when user recieves an SMS

Environmental Variables
Device: Buri v 1.1.0 Mozilla RIL
Build ID: 20131003004003
Gecko: http://hg.mozilla.org/releases/mozilla-aurora/rev/7dfe4a775531
Gaia: 9e21b6bea92fdafcb6787120a8cde0eb25a50495
Platform Version: 26.0a2
Firmware Version: US_20130912

Notes:
Repro frequency: 100%
Link to failed test case: https://moztrap.mozilla.org/manage/cases/?filter-id=1423
See attached: logcat

Comment 1

[Natalya Kot [:nkot]]

it seems like it requires to reboot device every time after changing Sound Settings in order to make it work... after rebooting I can hear selected Alert for incoming SMS until the next change in Sound Settings

Comment 2

[Natalya Kot [:nkot]]

Regression range:

-Does NOT Reproduce-
Gaia   9b1b262e8fde58be453fb05ed91c0e93ab86d394
SourceStamp 0322470077b7
BuildID 20130918004001
Version 26.0a2

-Reproduces-
Gaia   7b6147372cbf560744a02be50e0a862a825caef6
SourceStamp 0ff4dcd55258
BuildID 20130920122927
Version 26.0a2

*the issue started happening when Settings/Sound structure been changed
from: Settings/Sound-Tones/Ringer/Ringtones&Alerts_tones
to:   Settings/Sound-Tones/Ringer&Alerts

Comment 3

[Ivan Tsay (:ITsay)]

Tim, could your find engineer to help on this case. Thank you!

Comment 4

[Tim Guan-tin Chien [:timdream] (please needinfo)]

(In reply to nkot from comment #2)
> *the issue started happening when Settings/Sound structure been changed
> from: Settings/Sound-Tones/Ringer/Ringtones&Alerts_tones
> to:   Settings/Sound-Tones/Ringer&Alerts

According to the comment here it look like bug 905856 break this.

Comment 5

[David Flanagan [:djf]]

I tested that patch with ringtones, but not with the alert tone.

I broke it, so I own it.

Comment 6

[Ivan Tsay (:ITsay)]

Hi David,

Thank you for helping on this issue. Would you please assign one of the target milestones (1.2 C3 ~ 1.2 C6) flag for this bug. We want to use the flag to check when the bug will be resolved. Thank you!

Ivan

Comment 7

[David Flanagan [:djf]]

This was hard to track down.  It looks like this bug is a side effect of my workaround for bug 914404.  If I remove that workaround this bug goes away.  And, the crash I had seen in 914404 does not come back, at least on master: it appears that that bug is fixed.

I'll check that the workaround can safely be removed on aurora, and if so, that should make this bug go away.  Hopefully I'll have a patch for review tomorrow.

Comment 8

[David Flanagan [:djf]]

Attachment: link to github pr

Aus: I'm asking for your review because this patch makes a change to apps/system/js/notifications.js and you were the last to modify that file.

The main point of this patch is to remove a workaround for 914404. That touches the settings and ringtones apps, but should be trivial to review. (It touches the manifest files because the workaround required device storage permissions, and this patch now reduces those permisssions again).

The change to the system app (and a matching change to the sms app) just replace ringtonePlayer.src = '' with two lines of code that do the same thing in a cleaner way.  The existing code causes an error in logcat every time the notification sound is played.  The new code does not.

Comment 9

[Aus Lacroix [:aus]]

Comment on attachment 822589 [details] [review]
link to github pr

The changes look good to me, however, the travis build failed. Looks like unrelated failures but I'd recommend making sure it builds and passes all tests before landing (as much as that is possible, I know it can get a little messy at times.).

Comment 10

[David Flanagan [:djf]]

Travis is green now.

Landed to master: https://github.com/mozilla-b2g/gaia/commit/8db3fca246d8ba79a8bdd9998f5e12ed11533fea

Comment 11

[David Flanagan [:djf]]

932379 needs this fix too, so I'm going to try to do the uplift myself.

Comment 12

[David Flanagan [:djf]]

It needed a manual merge anyway.

Uplifted to v1.2: https://github.com/mozilla-b2g/gaia/commit/6b60ea5d7b5272126ae241d23e101b708528f2ab

Comment 13

[Shilpa Suresh]

This issue no longer reproduces on Buri v 1.2 11/07 build.

The changed alert tone is heard when user recieves an SMS.

Environmental Variables
Device: Buri v 1.2.0 Mozilla RIL
Build ID: 20131107004003
Gecko: http://hg.mozilla.org/releases/mozilla-b2g26_v1_2/rev/26f1e160e696
Gaia: 590eb598aacf1e2136b2b6aca5c3124557a365ca
Platform Version: 26.0
Firmware Version: US_20131104