1102814 - Make the dialer use the `notification' channel for playing the touch tones

Status: RESOLVED FIXED

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

Description

[Gabriele Svelto [:gsvelto]]

+++ This bug was initially created as a clone of Bug #1092362 +++

The dialer is currently using the `content' channel to play touch-tones as a workaround for bug 1092346. As soon as that bug gets fixed we should make it use the `notification' channel again. This is a clone of the original bug because we had to deploy a quick workaround for it due time constraints.

Comment 1

[GitHub Autolander Bot]

Attachment: [PullReq] gabrielesvelto:bug-1102814-keypad-volume-control-channel to mozilla-b2g:master

Comment 2

[Gabriele Svelto [:gsvelto]]

Comment on attachment 8559138 [details] [review]
[PullReq] gabrielesvelto:bug-1102814-keypad-volume-control-channel to mozilla-b2g:master

This patch restores the correct behavior and drops all the workarounds we had put in place for bug 1092346. I've tested this with the patch from that bug and the volume adjustment behavior is now correct.

Comment 3

[Tamara Hills [:thills]]

Comment on attachment 8559138 [details] [review]
[PullReq] gabrielesvelto:bug-1102814-keypad-volume-control-channel to mozilla-b2g:master

Hi Gabriele,

There is one test that is failing for me on contacts_test.js.  It's complaining about:     TypeError: navigator.mozAudioChannelManager is undefined
      at init (app://communications.gaiamobile.org/contacts/js/contacts.js:268:5)

Also, I am having trouble verifying the audio channel that is being used, but I'm just using console logging for this.  I'll ask you about this tomorrow.

Thanks,

-tamara

Comment 4

[Gabriele Svelto [:gsvelto]]

(In reply to Tamara Hills [:thills] from comment #3)
> There is one test that is failing for me on contacts_test.js.  It's
> complaining about:     TypeError: navigator.mozAudioChannelManager is
> undefined
>       at init
> (app://communications.gaiamobile.org/contacts/js/contacts.js:268:5)

Right, I forgot the mozAudioChannelManager is not available in desktop b2g. I'll make its use conditional.

> Also, I am having trouble verifying the audio channel that is being used,
> but I'm just using console logging for this.  I'll ask you about this
> tomorrow.

You can just look at the icon that appears when you adjust the volume. The `notification' channel has a phone icon, while the `content' channel has a speaker one.

I've pushed a patch on top of the PR with the changes to pass the tests.

Comment 5

[Tamara Hills [:thills]]

Comment on attachment 8559138 [details] [review]
[PullReq] gabrielesvelto:bug-1102814-keypad-volume-control-channel to mozilla-b2g:master

Looks good to me.

Comment 6

[Gabriele Svelto [:gsvelto]]

Thanks for the review. The try run was hopelessly riddled with intermittent failures so I've rebased my PR and pushed again to trigger a new run. Let's see what happens.

Comment 7

[Gabriele Svelto [:gsvelto]]

I had to retrigger some suites but try is finally green:

https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=6dc4d75c9ff1

Comment 8

[GitHub Autolander Bot]

Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.

Comment 9

[GitHub Autolander Bot]

Autolander could not locate a review from a user within the suggested reviewer list. Either the patch author or the reviewer should be in the suggested reviewer list.

Comment 10

[Gabriele Svelto [:gsvelto]]

Third try, now the suggested reviewer list should have been updated.

Comment 11

[GitHub Autolander Bot]

Pull request has landed in master: https://github.com/mozilla-b2g/gaia/commit/cbc34eae82f514cabd3366a721bf6b08a74b4fb0

Comment 12

[Johan Lorenzo [:jlorenzo]]

While testing bug 1129339, I came across an issue related to this patch: The DTMF tones can't be regulated anymore with the Volume control buttons. I reverted this patch locally and made the tones work again.

I backed this patch out before it goes through the next smoketests: https://github.com/mozilla-b2g/gaia/commit/479ac3e27a8e8fa9e2d102ef5f4e7d5f320210c5

Comment 13

[Gabriele Svelto [:gsvelto]]

(In reply to Johan Lorenzo [:jlorenzo] (QA) from comment #12)
> While testing bug 1129339, I came across an issue related to this patch: The
> DTMF tones can't be regulated anymore with the Volume control buttons. I
> reverted this patch locally and made the tones work again.

Could you elaborate on the problem? When you opened the dialer which volume control channel was active? The 'telephony' one (phone icon in the volume scroller) or the 'content' one (speaker icon in the volume scroller). I did some testing on my phone and the problem seems weirdly intermittent. Sometimes I get the right channel and sometimes I don't.

Comment 14

[Gabriele Svelto [:gsvelto]]

It's worse than I thought and I fear it might be some fallout from bug 1092346; with my patch applied I could consistently get an issue with the following STR:

1. Apply my patch, then re-build and flash with 'make reset-gaia' so that the phone gets the permission changes
2. Open the dialer, adjust the volume, the telephony channel is being used (OK)
3. Go back to the homescreen
4. Open the SMS app, adjust the volume, the content channel is being used (KO)

Since the SMS app explicitly requests the 'notification' channel like my patch here does I believe the two issues might be related.

Comment 15

[Gabriele Svelto [:gsvelto]]

OK, after more testing I think I've confirmed that bug 1092346 is causing the problem.

Comment 16

[Gabriele Svelto [:gsvelto]]

Adding a dependency to the new bug filed to fix the root cause. I'll wait for it to land then re-land this after testing.

Comment 17

[GitHub Autolander Bot]

Attachment: [gaia] gabrielesvelto:bug-1102814-keypad-volume-control-channel > mozilla-b2g:master

Comment 18

[Gabriele Svelto [:gsvelto]]

Try is green, ready to re-land: https://treeherder.mozilla.org/#/jobs?repo=gaia-try&revision=0343fe6b311c

Comment 19

[Gabriele Svelto [:gsvelto]]

Re-landed in gaia/master 0f2dbade8a410101ef3c6b7ca804c4f8f2c6ad2a

https://github.com/mozilla-b2g/gaia/commit/0f2dbade8a410101ef3c6b7ca804c4f8f2c6ad2a