Closed
Bug 1021550
Opened 9 years ago
Closed 9 years ago
[Phone][Ring Tone] The ring tone is muted after first call used speaker.
Categories
(Firefox OS Graveyard :: RIL, defect)
Tracking
(blocking-b2g:2.0+, firefox31 wontfix, firefox32 fixed, firefox33 fixed, b2g-v1.4 unaffected, b2g-v2.0 verified, b2g-v2.1 verified)
People
(Reporter: wachen, Assigned: thills)
References
Details
(Keywords: regression, Whiteboard: [2.0-FL-bug-bash] ft:loop [planned-sprint])
Attachments
(5 files, 6 obsolete files)
Build Information Gaia d2cfef555dabab415085e548ed44c48a99be5c32 Gecko https://hg.mozilla.org/mozilla-central/rev/51b428be6213 BuildID 20140605040202 Version 32.0a1 ro.build.version.incremental=96 ro.build.date=Fri May 23 11:17:40 CST 2014 Description If you picked up a phone and put it in a speaker mode, after hung up the phone call. The second phone call dialed in. It will either be no sound or only ring once. Steps to Reproduce 1. One phone dialed to target device(flame) 2. Target device picks the phone call up 3. Target device puts the phone call on speaker mode 4. Target device hangs up the phone 5. Another phone call dialed to target device Expected Results The ring tone goes on till the limited time or whenever people pick it up Actual Results The ring tone rings once or didn't ring at all. Other Notes Reproduction Frequency: 100%
Comment 1•9 years ago
|
||
QA Wanted to see if this issue is reproducible on 1.4.
User Story: (updated)
Keywords: qawanted
Updated•9 years ago
|
Component: Gaia → Gaia::Dialer
Comment 2•9 years ago
|
||
This issue does NOT occur on the Flame 1.4: Environmental Variables: Device: Flame 1.4 Build ID: 20140605145901 Gaia: 183efb374be1bcbf92f1fb3d0212a68b564a6d3e Gecko: 960c48b5a5fa Version: 30.0 (1.4) Firmware Version: v10G-2
Keywords: qawanted
QA Contact: ychung
Updated•9 years ago
|
blocking-b2g: --- → 2.0?
Keywords: regression,
regressionwindow-wanted
Updated•9 years ago
|
Updated•9 years ago
|
QA Contact: ychung → jmercado
Comment 3•9 years ago
|
||
B2g-inbound Regression Window Last working Environmental Variables: Device: Flame BuildID: 20140528233003 Gaia: eced604040a970e65dfb51fa83e1a18a9d00b7db Gecko: fe9a8825a3ae Version: 32.0a1 First Broken Environmental Variables: Device: Flame BuildID: 20140529075121 Gaia: d5dcbabd1ad07d5fd30a7875a9a80817bc93619c Gecko: f72106cb1769 Version: 32.0a1 Last working gaia / First broken gecko - Issue DOES occurs Gaia: eced604040a970e65dfb51fa83e1a18a9d00b7db Gecko: f72106cb1769 First broken gaia / Last working gecko - Issue does NOT occur Gaia: d5dcbabd1ad07d5fd30a7875a9a80817bc93619c Gecko: fe9a8825a3ae Gecko Pushlog: hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=fe9a8825a3ae&tochange=f72106cb1769
QA Whiteboard: [QAnalyst-Triage?]
Flags: needinfo?(jmitchell)
Updated•9 years ago
|
Keywords: regressionwindow-wanted
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage?] → [QAnalyst-Triage+]
Flags: needinfo?(jmitchell)
Comment 4•9 years ago
|
||
Corrected link for the Gecko Pushlog above: http://hg.mozilla.org/integration/b2g-inbound/pushloghtml?fromchange=fe9a8825a3ae&tochange=f72106cb1769
Comment 5•9 years ago
|
||
Maybe bug 1006380 broke this? Jose - What do you think?
Flags: needinfo?(josea.olivera)
Comment 6•9 years ago
|
||
I know Jose is pretty busy ;) @Maria could you manage to get feedback from him? Thanks a lot!
Flags: needinfo?(oteo)
Updated•9 years ago
|
QA Whiteboard: [QAnalyst-Triage+] → [QAnalyst-Triage+][lead-review+]
Comment 7•9 years ago
|
||
(In reply to Jason Smith [:jsmith] from comment #5) > Maybe bug 1006380 broke this? > > Jose - What do you think? Patch from bug 1006380 sets the audio phone state mode to PHONE_STATE_IN_COMMUNICATION when using the telephony audio channel and sets it back to PHONE_STATE_NORMAL when the app finishes using the audio channel. AFAIK There is no relationship with the ringer audio channel not being able to play correctly or just play once. Nevertheless I backed out the patch locally and the issue doesn't reproduce :( Better to have some feedback from Marco or even Andrea.
Flags: needinfo?(mchen)
Flags: needinfo?(josea.olivera)
Flags: needinfo?(amarchesini)
Updated•9 years ago
|
Flags: needinfo?(oteo)
Comment 8•9 years ago
|
||
Hi, This week, I am on the partner's office so I can't investigate it too much. I guess that ringer sound is going to receiver (earpiece). Maybe we can get clue from log of AudioManager::SetPhoneState, AudioManager::SetForceForUse and AudioManager::HandleAudioChannelProcessChanged
Flags: needinfo?(mchen)
Comment 9•9 years ago
|
||
(In reply to Marco Chen [:mchen] (Workshop with Partner 6/16 ~ 6/20) from comment #8) Thanks for your feedback Marco. > Maybe we can get clue from log of AudioManager::SetPhoneState, > AudioManager::SetForceForUse and > AudioManager::HandleAudioChannelProcessChanged You're right, I'll provide those logs as soon as possible.
Comment 10•9 years ago
|
||
(In reply to José Antonio Olivera Ortega [:jaoo] from comment #9) > (In reply to Marco Chen [:mchen] (Workshop with Partner 6/16 ~ 6/20) from > comment #8) > > Thanks for your feedback Marco. > > > Maybe we can get clue from log of AudioManager::SetPhoneState, > > AudioManager::SetForceForUse and > > AudioManager::HandleAudioChannelProcessChanged > > You're right, I'll provide those logs as soon as possible. There you go. If you take a look at the logs you see different behaviors. Without backing out patch from bug 1006380 AudioManager::HandleAudioChannelProcessChanged() method calls AudioManager::SetPhoneState(PHONE_STATE_IN_COMMUNICATION) when the user accepts the second incoming call while the ringtone is playing. It seems this causes the ringtone doesn't play correctly. Backing out the patch it doesn't happen, I mean AudioManager::HandleAudioChannelProcessChanged() method doesn't exist and there is no such call to AudioManager::SetPhoneState(PHONE_STATE_IN_COMMUNICATION) and everything works correctly. I've added the logs of a try in which the speakers are not enabled. If the speakers are enabled we see there is a call to AudioManager::SetForceForUse(aUsage(0), aForce(0)) when the device receives the second call and plays the ringtone. The call to SetForceForUse() seems to set the phone audio state to PHONE_STATE_NORMAL which causes AudioManager::HandleAudioChannelProcessChanged() calls AudioManager::SetPhoneState(PHONE_STATE_IN_COMMUNICATION). After noticing this I confirmed that the issue reported in this bug doesn't happen if the speakers are disabled before hanging up the call. Marco, this is what I see. Your input would be appreciate! Thanks. PS. Clearing the request at Andrea as we already know what happens here.
Attachment #8443075 -
Flags: feedback?(mchen)
Flags: needinfo?(amarchesini)
Comment 11•9 years ago
|
||
Hi :jaoo,
Thanks for your investigation and your comments inside file are useful to understand the logs.
> Devices receives a new gsm call
> I/AudioManager( 297): AudioManager::SetPhoneState(PHONE_STATE_RINGTONE)
> I/AudioManager( 297): AudioManager::SetForceForUse(aUsage(0), aForce(0))
> I/AudioManager( 297): AudioManager::SetPhoneState(PHONE_STATE_NORMAL)
As I know AudioManager::SetPhoneState is called by Telephony only, then we may need help from Hsin-Yi.
Hi Hsin-Yi,
It is strange that SetPhoneState will be called for PHONE_STATE_NORMAL during alerting state. Could you help to check it? Thanks.
Flags: needinfo?(htsai)
Comment 12•9 years ago
|
||
Hi all, I just quickly investigated this issue and it turns out that telephony set phone_state as expected. Telephony didn't set phone_state to NORMAL during alerting. Please see log snippet below: === 1st incoming call === 06-20 19:13:37.569 I/Gecko ( 498): -*- RadioInterface[0]: Received message from worker: {"rilMessageType":"callStateChange","call":{"state":4,"callIndex":1,"toa":129,"isMpty":false,"isMT":true,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"","numberPresentation":1,"name":"","namePresentation":1,"uusInfo":null,"isOutgoing":false,"isEmergency":false,"isConference":false},"rilMessageClientId":0} 06-20 19:13:37.569 I/Gecko ( 498): TelephonyService: handleCallStateChange: {"state":4,"callIndex":1,"toa":129,"isMpty":false,"isMT":true,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"","numberPresentation":1,"name":"","namePresentation":1,"uusInfo":null,"isOutgoing":false,"isEmergency":false,"isConference":false} === Set phone_state RINGTONE === 06-20 19:13:37.599 I/Gecko ( 498): TelephonyService: Put audio system into PHONE_STATE_RINGTONE: 1 === Accept the call === 06-20 19:13:42.859 I/Gecko ( 498): -*- RadioInterface[0]: Received message from worker: {"rilMessageType":"callStateChange","call":{"state":0,"callIndex":1,"toa":129,"isMpty":false,"isMT":true,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"","numberPresentation":1,"name":"","namePresentation":1,"uusInfo":null,"isOutgoing":false,"isEmergency":false,"isConference":false,"started":1403262822866},"rilMessageClientId":0} 06-20 19:13:42.859 I/Gecko ( 498): TelephonyService: handleCallStateChange: {"state":0,"callIndex":1,"toa":129,"isMpty":false,"isMT":true,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"","numberPresentation":1,"name":"","namePresentation":1,"uusInfo":null,"isOutgoing":false,"isEmergency":false,"isConference":false,"started":1403262822866} === Set phone_state IN_CALL === 06-20 19:13:42.869 I/Gecko ( 498): TelephonyService: Put audio system into PHONE_STATE_IN_CALL: 2 === Hang up the call === 06-20 19:13:51.179 I/Gecko ( 498): -*- RadioInterface[0]: Received message from worker: {"rilMessageType":"callDisconnected","call":{"state":0,"callIndex":1,"toa":129,"isMpty":false,"isMT":true,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"","numberPresentation":1,"name":"","namePresentation":1,"uusInfo":null,"isOutgoing":false,"isEmergency":false,"isConference":false,"started":1403262822866,"hangUpLocal":true,"failCause":"NormalCallClearingError"},"rilMessageClientId":0} 06-20 19:13:51.179 I/Gecko ( 498): TelephonyService: handleCallDisconnected: {"state":0,"callIndex":1,"toa":129,"isMpty":false,"isMT":true,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"","numberPresentation":1,"name":"","namePresentation":1,"uusInfo":null,"isOutgoing":false,"isEmergency":false,"isConference":false,"started":1403262822866,"hangUpLocal":true,"failCause":"NormalCallClearingError"} === Set phone_state NORMAL === 06-20 19:13:51.339 I/Gecko ( 498): TelephonyService: Put audio system into PHONE_STATE_NORMAL: 3 === 2nd incoming call === 06-20 19:14:04.989 I/Gecko ( 498): -*- RadioInterface[0]: Received message from worker: {"rilMessageType":"callStateChange","call":{"state":4,"callIndex":1,"toa":129,"isMpty":false,"isMT":true,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"","numberPresentation":1,"name":"","namePresentation":1,"uusInfo":null,"isOutgoing":false,"isEmergency":false,"isConference":false},"rilMessageClientId":0} 06-20 19:14:04.989 I/Gecko ( 498): TelephonyService: handleCallStateChange: {"state":4,"callIndex":1,"toa":129,"isMpty":false,"isMT":true,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"","numberPresentation":1,"name":"","namePresentation":1,"uusInfo":null,"isOutgoing":false,"isEmergency":false,"isConference":false} === Set phone_state RINGTONE === 06-20 19:14:05.019 I/Gecko ( 498): TelephonyService: Put audio system into PHONE_STATE_RINGTONE: 1 === Remote party released 2nd call === 06-20 19:14:14.609 I/Gecko ( 498): -*- RadioInterface[0]: Received message from worker: {"rilMessageType":"callDisconnected","call":{"state":4,"callIndex":1,"toa":129,"isMpty":false,"isMT":true,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"","numberPresentation":1,"name":"","namePresentation":1,"uusInfo":null,"isOutgoing":false,"isEmergency":false,"isConference":false,"failCause":"NormalCallClearingError"},"rilMessageClientId":0} 06-20 19:14:14.609 I/Gecko ( 498): TelephonyService: handleCallDisconnected: {"state":4,"callIndex":1,"toa":129,"isMpty":false,"isMT":true,"als":0,"isVoice":true,"isVoicePrivacy":false,"number":"","numberPresentation":1,"name":"","namePresentation":1,"uusInfo":null,"isOutgoing":false,"isEmergency":false,"isConference":false,"failCause":"NormalCallClearingError"} === Set phone_state NORMAL === 06-20 19:14:14.729 I/Gecko ( 498): TelephonyService: Put audio system into PHONE_STATE_NORMAL: 0
Flags: needinfo?(htsai)
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → thills
Assignee | ||
Comment 13•9 years ago
|
||
I also noticed the same behaviour as Jose where if I have the following sequence: 1) answer first call 2) go on speaker 3) go off speaker 4) hangup 5) make second inbound call Then everything rings properly. Based on that and the feedback from Hsin-Yi that the phone_states are as expected, I made a change to the end() function in calls_handler.js to check if the speaker is enabled before hangup and if it is, just mark speakerEnabled = false. That fixed it for me (Hamachi). Perhaps this is not the best place to fix it so I'll keep looking to see if there is another place where it should be turned off.
Assignee | ||
Comment 14•9 years ago
|
||
This is a log without the INCOMMUNICATION change and with it. I also filtered out the rpc_snd_set_device calls and noticed that with the change as soon as the INCOMMUNICATION state is set, it's sending an extra call to rpc_snd_set_device (which is not there without the patch). Looks like this call to rpc_send_set_device(0,1,0) can be changing the output. I'm trying to track back from the Audiohardware back to the AudioManager to see what's causing this.
Comment 15•9 years ago
|
||
Per discussion with Marco, we believe the root cause is in [1] and [2]. Bug 749794 added these two lines because there was a forced phone setting to IN_CALL or IN_COMMUNICATION to make speaker/microphone work, and we needed to set the phone state back to NORMAL if there's no active call (see bug 749794 comment 10). Then Bug 790570 removed the forced assignment without clearing [1] and [2]. Since after bug 790570, we don't force phone state to IN_CALL or IN_COMMUNICATION, we should be safe to remove [1] and [2] now. [1] http://dxr.mozilla.org/mozilla-central/source/dom/telephony/gonk/TelephonyService.js?from=TelephonyService.js&case=true#671 [2] http://dxr.mozilla.org/mozilla-central/source/dom/telephony/gonk/TelephonyService.js?from=TelephonyService.js&case=true#653
Comment 16•9 years ago
|
||
Hi Tamara, I think it's a gecko bug, please see comment 15. Not sure if you are still going to work on gecko side. If yes, I am happy to provide any assistance; if not, feel free to assign to me. Thank you. :)
Flags: needinfo?(thills)
Comment 17•9 years ago
|
||
Comment on attachment 8443075 [details] 1021550-log.txt Already give response in comment 11 and discussed with Hsin-Yi in comment 15
Attachment #8443075 -
Flags: feedback?(mchen)
Assignee | ||
Comment 18•9 years ago
|
||
Hi Hsin-Yi, I tried out your suggested change and it fixes it for me... If it's ok, I'd like to continue to work on it since I'm trying to get spun up in this area. If I could take your advice on what test suites I should run, that would be great.
Comment 19•9 years ago
|
||
(In reply to Tamara Hills from comment #18) > Hi Hsin-Yi, > > I tried out your suggested change and it fixes it for me... If it's ok, I'd > like to continue to work on it since I'm trying to get spun up in this area. No problem, all yours :) > If I could take your advice on what test suites I should run, that would be > great. We need to run marionette-webapi test. And I am thinking about to add a new marionette test to catch this issue. In that new test, we will need to call mozTelephony.speakerEnabled() API to set speaker enabled in different telephony call state. And we also need to check that toggling speakerEnabled doesn't change nsIAudioManager.phoneState. You could refer to dom/telephony/test/marionette/test_audiomanager_phonestate.js and /test_call_mute.js for some ideas! Let me know if anything I could help. :)
Updated•9 years ago
|
Target Milestone: --- → 2.0 S5 (4july)
Updated•9 years ago
|
Flags: needinfo?(thills)
Updated•9 years ago
|
Whiteboard: [2.0-FL-bug-bash] ft:loop → [2.0-FL-bug-bash] ft:loop [planned-sprint]
Assignee | ||
Comment 20•9 years ago
|
||
Hi Hsinyi, I'm seeing that the removing the check set the phone state back to normal when there are no active calls is breaking the test_audiomanager_callstate.js. Essentially, there is a check where it expects it to be in NORMAL state, but it's actually in RINGTONE (since we removed the code to set it to NORMAL). There is also this note in the test about 948860 and whether it should be expecting RINGTONE due to the UX implications. Wanted to get your feedback on changing this test as to whether there could be some other complication.
Attachment #8446943 -
Flags: feedback?(htsai)
Flags: needinfo?(htsai)
Comment 21•9 years ago
|
||
Comment on attachment 8446943 [details] [diff] [review] WIP 1021550 Review of attachment 8446943 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me!
Attachment #8446943 -
Flags: feedback?(htsai) → feedback+
Comment 22•9 years ago
|
||
(In reply to Tamara Hills from comment #20) > Created attachment 8446943 [details] [diff] [review] > WIP 1021550 > > Hi Hsinyi, I'm seeing that the removing the check set the phone state back > to normal when there are no active calls is breaking the > test_audiomanager_callstate.js. Essentially, there is a check > where it expects it to be in NORMAL state, but it's actually in RINGTONE > (since we removed the code to set it to NORMAL). There is also this note in > the test about 948860 and whether it should be expecting RINGTONE due to the > UX implications. > > Wanted to get your feedback on changing this test as to whether there could > be some other complication. I think bug 948860 is a duplicate of this bug. These two both are talking about the same thing. And your change to test_audiomanager_callstate.js makes sense.
Flags: needinfo?(htsai)
Comment 23•9 years ago
|
||
Comment on attachment 8446943 [details] [diff] [review] WIP 1021550 Review of attachment 8446943 [details] [diff] [review]: ----------------------------------------------------------------- Looks good to me! ::: dom/telephony/test/marionette/test_audiomanager_phonestate.js @@ +62,5 @@ > .then(call => { inCall = call; }) > // TODO - Bug 948860: should this be {RINGTONE, RINGTONE, RINGTONE}? > // From current UX spec, there is no chance that an user may enable speaker > // during alerting, so basically this 'set speaker enable' action can't > // happen in B2G. Once you fix this, please also remove this TODO comment.
Assignee | ||
Comment 25•9 years ago
|
||
Updated patch. Link to the try: https://tbpl.mozilla.org/?tree=Try&rev=9aea51f0774b
Attachment #8446943 -
Attachment is obsolete: true
Attachment #8448439 -
Flags: review?(htsai)
Assignee | ||
Updated•9 years ago
|
Status: NEW → ASSIGNED
Assignee | ||
Comment 26•9 years ago
|
||
Test case to handle the sequence of steps to repro this issue. Basically, * Receive an incoming call * Put the phone on speaker * Hang up without user turning off speaker * Receive second incoming call
Attachment #8448444 -
Flags: review?(htsai)
Comment 27•9 years ago
|
||
Comment on attachment 8448439 [details] [diff] [review] 1021550 Review of attachment 8448439 [details] [diff] [review]: ----------------------------------------------------------------- Looks great, thank you. The commit message isn't right for check-in. Please don't forget to modify the commit comment according to https://developer.mozilla.org/en-US/docs/Mozilla/Projects/L20n/Contribute#Adjust_the_commit_message
Attachment #8448439 -
Flags: review?(htsai) → review+
Comment 28•9 years ago
|
||
Comment on attachment 8448444 [details]
test_incomingcall_phonestate_speaker.js
This is not a patch format. Please upload a correct one again. Also, after adding a new test case, you will need to revise a corresponding manifest.ini, e.g. dom/telephony/test/marionette/manifest.ini, correspondingly.
Attachment #8448444 -
Flags: review?(htsai) → review-
Updated•9 years ago
|
Component: Gaia::Dialer → RIL
Assignee | ||
Comment 29•9 years ago
|
||
Hi Hsin-Yi, I put everything together in one patch file (including the test and the update to the manifest.ini). Let me know if that's the correct process. Also, I updated the commit message... I can update it again once I have your review to show that you reviewed. Thank you!
Attachment #8448439 -
Attachment is obsolete: true
Attachment #8448444 -
Attachment is obsolete: true
Attachment #8448709 -
Flags: review?(htsai)
Comment 30•9 years ago
|
||
Comment on attachment 8448709 [details] [diff] [review] Updated patch with test included and new commit msg. Review of attachment 8448709 [details] [diff] [review]: ----------------------------------------------------------------- This looks really good! Only some minor comments and nits. I'll give r+ after they are addressed, thank you for the work :) ::: dom/telephony/test/marionette/test_incomingcall_phonestate_speaker.js @@ +16,5 @@ > + > +let audioManager; > + > +function checkSpeakerEnabled(phoneStatePrev, phoneStateNew, toggle, setSpeaker) { > + if (!audioManager) { Use 2-space indentation over the file. @@ +21,5 @@ > + audioManager = SpecialPowers.Cc[AUDIO_MANAGER_CONTRACT_ID] > + .getService(SpecialPowers.Ci.nsIAudioManager); > + ok(audioManager, "nsIAudioManager instance"); > + } > + You got two empty lines here. Please remove one. @@ +23,5 @@ > + ok(audioManager, "nsIAudioManager instance"); > + } > + > + > + ok(audioManager.phoneState === phoneStatePrev, "audioManager.phoneState"); Let's use |is(audioManager.phoneState, phoneStatePrev, "audioManager.phoneState");| @@ +27,5 @@ > + ok(audioManager.phoneState === phoneStatePrev, "audioManager.phoneState"); > + if(toggle) { > + telephony.speakerEnabled = setSpeaker; > + } > + ok(audioManager.phoneState === phoneStateNew, "audioManager.phoneState"); ditto. @@ +45,5 @@ > + .then(() => checkSpeakerEnabled(PHONE_STATE_RINGTONE, PHONE_STATE_RINGTONE, false, false)) > + .then(() => gAnswer(inCall)) > + .then(() => checkSpeakerEnabled(PHONE_STATE_IN_CALL, PHONE_STATE_IN_CALL, false, false)) > + // Go on Speaker (Don't go off speaker before hanging up. This is to check > + // the condition for 1021550) nit: s/1021550/bug 1021550/ @@ +52,5 @@ > + .then(() => gRemoteHangUp(inCall)) > + .then(() => checkSpeakerEnabled(PHONE_STATE_NORMAL, PHONE_STATE_NORMAL, false, false)) > + // Make a second inbound call > + .then(() => gRemoteDial(inNumber)) > + .then(call => { inSecondCall = call; }) We could reuse the variable 'inCall' as we don't have two calls at the same time. Please remove the variable 'inSecondCall' thanks!
Attachment #8448709 -
Flags: review?(htsai)
Assignee | ||
Comment 31•9 years ago
|
||
Hi Hsin-Yi, thanks for the review. I think I've got everything in this one now. Thanks.
Attachment #8448709 -
Attachment is obsolete: true
Attachment #8449371 -
Flags: review?(htsai)
Comment 32•9 years ago
|
||
Comment on attachment 8449371 [details] [diff] [review] Updates to the patch with Hsin-Yi's last comments Review of attachment 8449371 [details] [diff] [review]: ----------------------------------------------------------------- Looks great! Thank you for the work. r=me with the comment addressed. If you don't have level-3 privilege, please upload a final patch with all required information in commit message and put keyword "checkin-needed" so that anyone authorized could help you land the patch. Thank you again :) ::: dom/telephony/test/marionette/test_incomingcall_phonestate_speaker.js @@ +55,5 @@ > + .then(() => checkSpeakerEnabled(PHONE_STATE_RINGTONE, PHONE_STATE_RINGTONE, false, false)) > + .then(() => gAnswer(inCall)) > + .then(() => checkSpeakerEnabled(PHONE_STATE_IN_CALL, PHONE_STATE_IN_CALL, false, false)) > + // Hang up the call > + .then(() => gRemoteHangUpCalls([inCall])) Using gRemoteHangUp(inCall) is enough.
Attachment #8449371 -
Flags: review?(htsai) → review+
Assignee | ||
Comment 33•9 years ago
|
||
Final patch which needs checkin. There is an updated try run as well: https://tbpl.mozilla.org/?tree=Try&rev=4ca1e399b579
Attachment #8449371 -
Attachment is obsolete: true
Assignee | ||
Comment 34•9 years ago
|
||
Added the checkin-needed in the commit message.
Attachment #8451204 -
Attachment is obsolete: true
Comment 35•9 years ago
|
||
(In reply to Tamara Hills [:thills] from comment #34) > Created attachment 8451205 [details] [diff] [review] > Final patch -- added checkin-needed in the commit message > > Added the checkin-needed in the commit message. Hey, Tamara, Thanks for the final patch :) Just want to double confirm: is this patch ready for check-in? If yes, I could do that for you. But next time, please add "checkin-needed" in "Keywords" plank! Sorry that I seemed not explain clear enough :)
Flags: needinfo?(thills)
Assignee | ||
Comment 36•9 years ago
|
||
Hi Hsin-Yi, Yes, it's ready. That would be great. Thanks, -tamara
Assignee | ||
Updated•9 years ago
|
Flags: needinfo?(thills)
Keywords: checkin-needed
Comment 37•9 years ago
|
||
https://hg.mozilla.org/integration/b2g-inbound/rev/90fde19681b9
Keywords: checkin-needed
Comment 38•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/90fde19681b9
Status: ASSIGNED → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Comment 39•9 years ago
|
||
https://hg.mozilla.org/releases/mozilla-aurora/rev/4272b20e0667
status-b2g-v2.1:
--- → fixed
status-firefox31:
--- → wontfix
status-firefox32:
--- → fixed
status-firefox33:
--- → fixed
Target Milestone: 2.0 S5 (4july) → 2.0 S6 (18july)
Comment 40•8 years ago
|
||
This issue has been verified successfully on Flame 2.0 &2.1. See attachment: Verify_Video_Flame.MP4 Reproducing rate: 0/10 Flame v2.0 version: Gaia-Rev 8d1e868864c8a8f1e037685f0656d1da70d08c06 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g32_v2_0/rev/c756bd8bf3c3 Build-ID 20141202000201 Version 32.0 Flame v2.1 version: Gaia-Rev ccb49abe412c978a4045f0c75abff534372716c4 Gecko-Rev https://hg.mozilla.org/releases/mozilla-b2g34_v2_1/rev/18fb67530b22 Build-ID 20141202001201 Version 34.0
Updated•8 years ago
|
You need to log in
before you can comment on or make changes to this bug.
Description
•