822318 - Silent mode doesn't silence received-text chime

Status: VERIFIED FIXED

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

Description

[Justin Lebar (not reading bugmail)]

STR:

* Hold down the power button.
* Select "silence incoming calls".
* Receive a text message.

Actual results:

* Chime plays

Expected results:

* No chime plays, because we're in silent mode.

Comment 1

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch

Comment 2

[Etienne Segonzac (:etienne)]

Comment on attachment 693371 [details] [diff] [review]
Patch

Review of attachment 693371 [details] [diff] [review]:
-----------------------------------------------------------------

Using a global setting instead of trying to maintain a list of settings to toggle is definitely the way to go.

Having a silent-mode.enabled setting would have made things a bit clearer but I don't think it's worth the hassle that a setting rename would cause dogfooders.

Comment 3

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

Attachment: Patch v2

I found a case where the sound was still here on an incoming call even if it was disabled... I also add back the indicators in the statusbar/lockscreen.

Comment 4

[Etienne Segonzac (:etienne)]

Comment on attachment 693492 [details] [diff] [review]
Patch v2

Review of attachment 693492 [details] [diff] [review]:
-----------------------------------------------------------------

Apparently my android phone has the same weird UX, so not holding the review for it :)

r=me with the nits addressed

::: apps/communications/dialer/js/oncall.js
@@ +181,1 @@
>      activePhoneSound = !!value;

nit: we get a boolean now, no need for |!!|

@@ +301,5 @@
>          });
>        } else {
> +        if (activateVibration) {
> +          navigator.vibrate([100, 100, 100]);
> +        }

nit: maybe start with the |if (activeVibration)| and not even do the mozvisibilitychange dance if false.

::: apps/system/js/sleep_menu.js
@@ +38,5 @@
> +
> +    var settings = navigator.mozSettings;
> +    SettingsListener.observe('audio.volume.notification', 7, function(value) {
> +      settings.createLock().set({'ring.enabled': (value != 0)});
> +    });

Just want to confirm the user experience:

* put the phone in silent mode with the sleep button menu
* press one of the volume button (changing the volume value)
-> I get out of silent mode?

Comment 5

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

(In reply to Etienne Segonzac (:etienne) from comment #4)
> Comment on attachment 693492 [details] [diff] [review]
> Patch v2
> 
> Review of attachment 693492 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Apparently my android phone has the same weird UX, so not holding the review
> for it :)
> 
> r=me with the nits addressed
> 
> ::: apps/communications/dialer/js/oncall.js
> @@ +181,1 @@
> >      activePhoneSound = !!value;
> 
> nit: we get a boolean now, no need for |!!|
> 
> @@ +301,5 @@
> >          });
> >        } else {
> > +        if (activateVibration) {
> > +          navigator.vibrate([100, 100, 100]);
> > +        }
> 
> nit: maybe start with the |if (activeVibration)| and not even do the
> mozvisibilitychange dance if false.

I want the vibration to turn on when the setting returns. Maybe I should add a comment.

> 
> ::: apps/system/js/sleep_menu.js
> @@ +38,5 @@
> > +
> > +    var settings = navigator.mozSettings;
> > +    SettingsListener.observe('audio.volume.notification', 7, function(value) {
> > +      settings.createLock().set({'ring.enabled': (value != 0)});
> > +    });
> 
> Just want to confirm the user experience:
> 
> * put the phone in silent mode with the sleep button menu
> * press one of the volume button (changing the volume value)
> -> I get out of silent mode?

Yep.

Comment 6

[Etienne Segonzac (:etienne)]

(In reply to Vivien Nicolas (:vingtetun) from comment #5)
> > @@ +301,5 @@
> > >          });
> > >        } else {
> > > +        if (activateVibration) {
> > > +          navigator.vibrate([100, 100, 100]);
> > > +        }
> > 
> > nit: maybe start with the |if (activeVibration)| and not even do the
> > mozvisibilitychange dance if false.
> 
> I want the vibration to turn on when the setting returns. Maybe I should add
> a comment.
> 

Nope, my bad, I thought I was commenting notifications.js where we don't have a setInterval so no occasion to have activateVibration becoming true.

Comment 7

[Vivien Nicolas (:vingtetun) (:21) - (NOT reading bugmails, needinfo? please)]

https://github.com/mozilla-b2g/gaia/commit/81fc5ce34ce63d6ad18058473973887ad79d9b59

Comment 8

[mdaniloff]

Verified on Unagi build #20121231070201