Closed
Bug 1092362
Opened 10 years ago
Closed 10 years ago
Make the dialer use the `notification' channel for playing the touch tones
Categories
(Firefox OS Graveyard :: Gaia::Dialer, defect)
Tracking
(blocking-b2g:2.2+, b2g-v2.2 fixed)
Tracking | Status | |
---|---|---|
b2g-v2.2 | --- | fixed |
People
(Reporter: gsvelto, Assigned: gsvelto)
References
Details
Attachments
(2 files, 1 obsolete file)
+++ This bug was initially created as a clone of Bug #834530 +++ 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.
Comment 2•10 years ago
|
||
Carrying over the nomination from the duplicate (Bug 1098426 - Music stops brutally when you press a key on the keypad) [Blocking Requested - why for this release]: Audible regression with a high user impact.
blocking-b2g: --- → 2.2?
Updated•10 years ago
|
Assignee: nobody → gsvelto
Target Milestone: --- → 2.1 S9 (21Nov)
Assignee | ||
Comment 6•10 years ago
|
||
This is a quick and safe workaround that makes the tones use the default audio channel. The default and content channels share the same volume control so the keypad behavior is unchanged; the upside is that the default channel doesn't cause the play icon to appear in the status bar nor does it interrupt applications explicitly using the content channel. I've tested that the case were music was interrupted by typing in the keypad works correctly with this patch. This isn't a proper fix because a proper fix requires far more investigation than I have time for at this stage.
Attachment #8524641 -
Flags: review?(drs.bugzilla)
Assignee | ||
Updated•10 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•10 years ago
|
||
Comment 8•10 years ago
|
||
Comment on attachment 8524641 [details] [diff] [review] [PATCH] Use the default audio channel when playing audible tones outside of calls Review of attachment 8524641 [details] [diff] [review]: ----------------------------------------------------------------- Why is the Emergency Call app still using the 'notification' channel? r- for this but please re-request review if you have an explanation. ::: apps/communications/dialer/test/unit/keypad_test.js @@ +108,5 @@ > function() { > this.sinon.spy(MockTonePlayer, 'init'); > KeypadManager.init(/* oncall */ false); > > + sinon.assert.calledWith(MockTonePlayer.init); s/calledWith/calledOnce/ ::: shared/js/dialer/tone_player.js @@ +16,5 @@ > _channel: null, > _gainNode: null, > _playingNodes: [], > _maybeTrashAudio: null, > + _initialized: false, We should test this functionality. @@ +82,4 @@ > return; > } > > + if (this._channel) { I think this would work as well: ```js var channel = this._channel ? this._channel : undefined; this._audioContext = new AudioContext(channel); ```
Attachment #8524641 -
Flags: review?(drs.bugzilla) → review-
Assignee | ||
Comment 9•10 years ago
|
||
(In reply to Doug Sherk (:drs) (use needinfo?) from comment #8) > Why is the Emergency Call app still using the 'notification' channel? r- for > this but please re-request review if you have an explanation. Using the `notification' channel in the emergency call app works. It's only in the dialer that the channel switching is borked. > s/calledWith/calledOnce/ I used calledWith() in the hope that it will check that the call was made w/o any arguments. I'm not sure if that's the case though. > We should test this functionality. It should be covered by the unit-test that ensure nothing happens before initialization though with your comment below you gave me the idea that I don't need that extra field. Let me refresh the patch.
Assignee | ||
Comment 10•10 years ago
|
||
(In reply to Gabriele Svelto [:gsvelto] from comment #9) > I used calledWith() in the hope that it will check that the call was made > w/o any arguments. I'm not sure if that's the case though. Replying to myself, what I was looking for is calledWithExactly(). It can be coupled with calledOnce() to ensure that we call the method only once and with those parameters alone.
Assignee | ||
Comment 11•10 years ago
|
||
Updated patch with review comments taken into account. Unfortunately your suggestion of setting the channel explicitly to |undefined| doesn't work so I left that part unchanged. I've also updated the PR by pushing the new changes on top of the existing ones.
Attachment #8524641 -
Attachment is obsolete: true
Attachment #8525609 -
Flags: review?(drs.bugzilla)
Comment 12•10 years ago
|
||
Comment on attachment 8525609 [details] [diff] [review] [PATCH v2] Use the default audio channel when playing audible tones outside of calls Review of attachment 8525609 [details] [diff] [review]: ----------------------------------------------------------------- Looks good. We'll need a Contacts peer to review this, so I'm asking Francisco.
Attachment #8525609 -
Flags: review?(francisco)
Attachment #8525609 -
Flags: review?(drs.bugzilla)
Attachment #8525609 -
Flags: review+
Comment 13•10 years ago
|
||
Please make a demo and put it on the sprint demos page when you get a chance: https://wiki.mozilla.org/FirefoxOS/Comms/Dialer/Sprint/v2.1-S9#Demos
Flags: needinfo?(gsvelto)
Comment 14•10 years ago
|
||
Comment on attachment 8525609 [details] [diff] [review] [PATCH v2] Use the default audio channel when playing audible tones outside of calls r+ for the contacts part.
Attachment #8525609 -
Flags: review?(francisco) → review+
Updated•10 years ago
|
QA Contact: jlorenzo
Assignee | ||
Comment 15•10 years ago
|
||
Try was finally green after having re-triggered an intermittent we should fix soon: https://treeherder.mozilla.org/ui/#/jobs?repo=gaia-try&revision=f2bd9b1bb3f4 Pushed to gaia/master e327daf01fac1e5fbe2ba732c26e3a21db32443a https://github.com/mozilla-b2g/gaia/commit/e327daf01fac1e5fbe2ba732c26e3a21db32443a
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Flags: needinfo?(gsvelto)
Resolution: --- → FIXED
Updated•10 years ago
|
status-b2g-v2.2:
--- → fixed
You need to log in
before you can comment on or make changes to this bug.
Description
•