Let FM radio can output sound from speaker. ToDo: 1. add a SpeakerOn button on fmRaidio app 2. add api on fmradio 3. add api on AudioManager, switch audio patch between SND_DEVICE_FM_SPEAKER/SND_DEVICE_FM_HEADSET

Invoke the setForceUse function call in AudioManager for switching speaker on/off. So we don't need to add interface on AudioManager idl.

Hi Sicking, For this case, should we add this DOM api on fmradio?

I believe we talked about this when we were implementing the FM radio, and we decided this API was inappropriate. I think we should have an API which says "direct audio to the speaker", not an API which says "direct the FM radio to the speaker". I'm not sure where the API I'm proposing should live, though.

new API for turn-on speaker on AudioChannelManager.webidl

Hi Justin, I add the speaker control function on AudioChannelManager, This may require 1. patch for switch between apps which is playing. ex: fm app->music app. The music app should playing through headset, not speaker. 2. permission?

triage: not blocking leo for now. We are interested in this feature but have not yet committed it to a specific release.

Comment on attachment 730578 [details] [diff] [review] wip Randy, I'm sorry I took so long to look at this patch. Jonas and Andrea own this API; I don't feel comfortable reviewing these changes.

Hi Casey, Do we have any user story about this feature? We need this before design new DOM API.

Hi Randy, Sorry for the late reply. We have UX specs drafted up for this here at bug 863098.

I think it'd be better to have an API which explicitly lets an app use headphones just as an antenna and not for audio. So something like: DOMRequest setHeadphonesDisabled(boolean); DOMRequest getHeadphonesDisabled(); When headphones-disabled is set to true we play audio the way we would if the headphones weren't plugged in.

Hi Jonas, The name of this API seems to imply that headphone is disabled now so please choose to other output device based on policy in your OS. I think it would be a right result on FM app because speaker is the output device after disabling headphone. But there is also a scenario about forcing speaker on in dialer app - during a call user can set to speakerOn mode for listening remote voice via speaker. If we use setHeadphonesDisabled(true) then output device will be earphone not speaker.

I think we have a couple of use cases here: 1. The user should be able to go to the radio app and plug in a pair of headphones to use as antenna. He should then be able to press a button in the radio app to indicate that the audio should not go to the headphones, but rather to their normal audio source. I.e. audio from the radio should come from the speaker, audio from games should come from the speaker, and audio from the telephone should come from the earphone. In this scenario I think it's also important that if the user unplugs the headphones and then plugs them back in, the audio now goes to the headphones. I.e. it's important that there's no persistent setting which the user has to figure out how to disable in order to get the headphones to work normally again. 2. User should be able to go a dialer app, such as skype or the built-in dialer, and make a phone call. He should then be able to press a button in that dialer app to have the audio start coming out of speaker rather than the earphone. It appears that the API in this patch is able to handle second use case well. However calling setspeakeron(true, "media") would still cause telephony audio to go to the headphones, and so there's no way to satisfy use case 1.

Hi Jonas, For the API word meaning DOMRequest setHeadphonesDisabled(boolean); DOMRequest getHeadphonesDisabled(); If UA call the setHeadphonesDisabled(false), 1. What does the system act like? ie. all audio routing isn't go through the headphone/mic? 2. Do we want this API just affect this application or whole system?

My understanding is that we want to affect the whole system. I.e. the user should be able to go to the radio app, plug in headphones in order to use them as antenna, but not put the headphones in his ears. So when there's an incoming notification, alarm or phone call, the audio from those should not go to the headphones, but rather to the normal speaker. FWIW, I agree that the names I proposed aren't very good. Something like: DOMRequest setHeadphonesAntennaOnly(boolean); DOMRequest getHeadphonesAntennaOnly(); would be more descriptive.

To consider the use case as below, 1. FMRadio app calls setHeadphonesAntennaOnly(true), so audio is came from speaker now. Q1:Then a voice call is coming. In this time the voice is output to speaker too. Thus what Web API should be used by dialer app to disable "output to speaker" and back to headphone. a. AudioChannelManager::setHeadphonesAntennaOnly(false) b. Telephony::speakerEnabled = false; The web api for controlling output path now have Telephony::speakerEnabled AudioChannelManager::setHeadphonesAntennaOnly BT::SCOConnected/Disconnected Do we consider to put them into one category? Thanks.

When setHeadphonesAntennaOnly(true) has been called, it means that we'll do audio routing just as if the headphones weren't plugged in. So if there's an incoming phone call, the ringer is routed to the speaker. When the user answers the phone, the telephony audio goes to the earphone. If the user presses the "speaker" button in the dialer UI we call telephony.speakerEnabled=true which makes the telephony audio go to the speaker. I'd be happy to also change the telephony.speakerEnabled API such that it doesn't live on the telephony interface. Maybe something like AudioChannelManager.telephonySpeaker = true. But that's a separate discussion I think?

Sorry, I missed the question about BT::SCOConnected. I don't know how that works right now. Could you explain?

The BT::SCOConnected is used for dialer and when user switch to speaker during the phone call, the device should disconnect the HANDS-FREE PROFILE, otherwise the device can't pass the certification.

(In reply to Jonas Sicking (:sicking) from comment #17) > When setHeadphonesAntennaOnly(true) has been called, it means that we'll do > audio routing just as if the headphones weren't plugged in. > > So if there's an incoming phone call, the ringer is routed to the speaker. > When the user answers the phone, the telephony audio goes to the earphone. > If the user presses the "speaker" button in the dialer UI we call > telephony.speakerEnabled=true which makes the telephony audio go to the > speaker. > > I'd be happy to also change the telephony.speakerEnabled API such that it > doesn't live on the telephony interface. Maybe something like > AudioChannelManager.telephonySpeaker = true. But that's a separate > discussion I think? For the headphone, It sound like this API would change the system behavior, 1. Does this API should affect the headphone's icon on status bar? 2. If the FM application is closed/muted and switch to music, should we recover the headphone's status? User may need to unplug-plug the headphone and force audio patch switch back to headphone again..It's a little tricky.

Hi all, UX guy already provided a first version of wireframes on bug 863098 for this requirement. Maybe we can move discussion to there and UX can join to discuss.

Connecting 863098 to this formally so related bugs can be found outside of the comments.

Hi sicking, This is the SpeakerManager prototype for forcing audio path to Speaker, Would you kindly help to review the IDL part? I think this object should be implemented to be the parent/child type right?

Dylan Oliver deleted the linked story in Pivotal Tracker

Candice Serran deleted the linked story in Pivotal Tracker

Set it duplicate to Bug 863098. Forward to work on that.

Reopen it and change the title to Implement SpeakerManager API.

Hi Jonas, Here is my idea for this API https://docs.google.com/a/mozilla.com/drawings/d/1K8qKeKuD71tCpM8BF0q7CqplkMenDhM_dFf-uI8dlKg/edit

The api usage may like this var spkmgr = new SpeakerManager(); spkmgr.onforcespeakerchange = function () { console.log(spkmgr.speakerforced); } spkmgr.forcespeaker(true); // spkmgr.forcespeaker(false);

Hi Baku, If you have time, can you take a look for this one? :) This is a workable patch, refer to audiochannel design way.

Hi Jonas, Could you give some feedback about this? Or you prefer the threatHeadsetAsAntenna idea? Or expend it to be the audio output devices resource manager?

Comment on attachment 791853 [details] [diff] [review] WIP Review of attachment 791853 [details] [diff] [review]: ----------------------------------------------------------------- looks good so far. Sorry the delay! Please add some mochitests.

Comment on attachment 791853 [details] [diff] [review] WIP Review of attachment 791853 [details] [diff] [review]: ----------------------------------------------------------------- The API looks good to me. Though I would like it if we added a permission for this. No prompt needed or anything, but if we could add an entry in the permissions table and ask developers to enumerate the API in the manifest. That way we could allow users to control this permission through the settings app in the future. Also, whenever we have a setup like this with an attribute and an event that is fired whenever the attribute changes, we try to do have a single asynchronous action which does 1. Change the property 2. Fire the event I.e. we never return to the event loop between the two. That reduces the risk of application-level bugs where the application depends on the value of the property never changing unless the event fires. It's not entirely clear that that's enforced here. Definitely not a big deal in this API specifically, but something to keep in mind in these types of situations. ::: dom/system/gonk/SpeakerManagerService.cpp @@ +82,5 @@ > +SpeakerManagerService::ForceSpeaker(bool enable, uint64_t aChildid) { > + nsCOMPtr<nsIAudioManager> audioManager = do_GetService(NS_AUDIOMANAGER_CONTRACTID); > + NS_ENSURE_TRUE(audioManager, NS_OK); > + if (enable) { > + audioManager->SetForceForUse(nsIAudioManager::USE_MEDIA,nsIAudioManager::FORCE_SPEAKER); space after comma

And yes, definitely needs tests

Thanks Jonas and Andrea, I will provide new patch&test case as soon as possible.

Actually, I had some questions with this API after re-reading the comment in bug 863098 comment 28 and on. Will this API allow us to implement the UX in https://mozilla.app.box.com/s/6yiq70tfptzadbvenade ? It doesn't seem like that is the case. Unfortunately that draft is dramatically lacking in details which makes it sort of impossible to implement. Is there a later draft from the UX team? If the user presses the speaker button in the radio-app UI, I assume that we'd call SpeakerManager.forcespeaker(true). At what point would we stop forcing the speaker to be on? At the very least it seems like we'd need to stop forcing the speaker when the user quits the radio app. Do we stop forcing when the user presses the home button? What happens if App A calls SpeakerManager.forcespeaker(true), and then app B calls SpeakerManager.forcespeaker(false). Does the second call just mean that app B doesn't force the speaker to be on, but that since app A is still forcing speaker that it's still being forced on? Or does the second call remove the forced speaker for all apps? I'm guessing that's the case.

For this speaker Competition case, I think the simple rule is the later wins. a. The others APs which used this API should receive the onforcespeakerchange event and update the UI Status. But still need UX agree this design. For restore speaker status, if the AP crashed or closed or speakermanager object destroy, the SpeakerManagerService should aware this status change and restore it to original status (It seems we should maintain a childID - speakerstatus array in SpeakerManagerServer). For AP unexcept crash or close case. ex: AP1:ON AP2:OFF AP3:ON ->if AP1 crash, then device speaker status should be ON AP1:ON AP2:OFF AP3:OFF ->if AP1 crash, then device speaker status should be OFF AP1:OFF AP2:ON AP3:OFF ->if AP1 crash, then device speaker status should be ON ...

Another question is Do we want to let all apps use this API to control the speaker status, ie Telephony? Now the design is just for media type. But We can let this API to control the device speaker status whenever audio stream is FM/Media/Telephony/WebRTC.

(In reply to Randy Lin [:rlin] from comment #37) > For this speaker Competition case, I think the simple rule is the later wins. > a. The others APs which used this API should receive the > onforcespeakerchange event and update the UI Status. But still need UX agree > this design. > > For restore speaker status, if the AP crashed or closed or speakermanager > object destroy, the SpeakerManagerService should aware this status change > and restore it to original status (It seems we should maintain a childID - > speakerstatus array in SpeakerManagerServer). > For AP unexcept crash or close case. > ex: > AP1:ON AP2:OFF AP3:ON ->if AP1 crash, then device speaker status should be > ON > AP1:ON AP2:OFF AP3:OFF ->if AP1 crash, then device speaker status should be > OFF > AP1:OFF AP2:ON AP3:OFF ->if AP1 crash, then device speaker status should be > ON > ... Correct some upper comment problem. For the last win policy, think the UseCase AP1:SPK_OFF ->open AP2 and turn on SPK---->AP1:SPK_ON AP2:SPK_ON --->Open AP3 and turn off SPK----> AP1/2/3 SPK_OFF If one of the AP crash/closed and the speaker is turn_on, I think we should turn_off the speaker, ex: fm is playing and cause music pause, then FM app dead, the music audio will come out from speaker and NO UI to turn it back to headset.

Another idea is binding the speaker status with audiochannel. i.e If AP set speaker on but the stream is paused by some AP, the device's speaker status will become off, then turn back to on if the AP resume to playing. That can let AP to keep their speaker status and turn on when stream is playing.

1. Add mochitest test case, (Found memory leak, still debugging) 2. Add permission control: speaker-control I think now is usable for FMRadio to test speakerOn function. But some point may need UA to handle at this point. 1. FM may need to turn-off the speaker_on while stop it. 2. FM paused by audio channel, then music sound will output on speaker. This kind of issue I think can combine audiochannel services to monitor app's behavior and set correct speaker status.

The WebIDL for this function.

1. Fix memory leak 2. Fix speaker status transaction problem TODO: deal with speaker completion issue.

Newer patch, fix some OS build break. Next one would hook with audiochannel services and control the speaker status.

update interface name to MozSpeakerManager

Change interface name from SpeakerManager to MozSpeakerManager.

I still don't understand how this API would work. Say that the user starts the radio app and presses the speaker button. This causes the app to call manager.forcespeaker=true; If audio now plays from the fmradio, the audio comes out of the speaker. Say that the user now pauses the radio, but leaves the speaker button still pressed. The user then switches to a game which contains some sound effects. Does the audio still come from the speaker? If the user wants to use headphones, does the user have to switch back to the radio app and press the speaker button again so that manager.forcespeaker=false; is called? What happens if we run low on memory and the radio app is killed. Does the speaker suddenly turn off and we start using headphones?

(In reply to Jonas Sicking (:sicking) (vacation until 9/9. In norway until 9/16) from comment #53) > I still don't understand how this API would work. > > Say that the user starts the radio app and presses the speaker button. This > causes the app to call manager.forcespeaker=true; > > If audio now plays from the fmradio, the audio comes out of the speaker. I hook the speaker status with the audio channel, so fmradio is paused, that the speaker status would be turn off. Games sound won't play on speaker. > Say that the user now pauses the radio, but leaves the speaker button still > pressed. The user then switches to a game which contains some sound effects. > > Does the audio still come from the speaker? > > If the user wants to use headphones, does the user have to switch back to > the radio app and press the speaker button again so that > manager.forcespeaker=false; is called? User doesn't need to do it because the speaker status auto turn off if this APP stream has been pause/stop. > What happens if we run low on memory and the radio app is killed. Does the > speaker suddenly turn off and we start using headphones? I handle this case in Services side, If APP got killed by low memory or kill by user, That would trigger IPC shutdown and turn off the speaker if this APP's status is on.

Sorry, I still don't quite understand. 1. User plugs in headphones 2. User enters radio app 3. User presses speaker button 4. What does speakermanager.speakerforced inside of the radio app return? 5. User presses play button in radio app 6. Where does the radio audio come from? 7. User presses pause button 8. What does speakermanager.speakerforced inside of the radio app return? 9. Is the speaker button still rendered as pressed? 10. User presses home button 11. User launches music app (radio app still is running in the background but doesn't play radio) 12. What would speakermanager.speakerforce inside the music app return if the music app read it? 13. User press play in the music app 14. Where does the music audio come from? 15. User presses pause in the music app 16. User switches back to radio app 17. Does the speaker button still render as pressed? 18. What does speakermanager.speakerforced inside of the radio app return? What would the answers be to questions 12, 14, 17 and 18 be if the user entered a game which uses the "normal" channel instead of the music app.

actually, also add these steps 19. User kills the radio app 20. User enters music app again. 21. What would speakermanager.speakerforce inside the music app return if the music app read it? 22. User press play in the music app 23. Where does the music audio come from?

(In reply to Jonas Sicking (:sicking) (vacation until 9/9. In norway until 9/16) from comment #55) > Sorry, I still don't quite understand. > > 1. User plugs in headphones > 2. User enters radio app > 3. User presses speaker button I think user can't press this button...Reference android phone with FMRadio function > 4. What does speakermanager.speakerforced inside of the radio app return? The speakermanager.speakerforced would return true after the onforcespeakerchange event. > 5. User presses play button in radio app > 6. Where does the radio audio come from? Speaker > 7. User presses pause button > 8. What does speakermanager.speakerforced inside of the radio app return? False > 9. Is the speaker button still rendered as pressed? SpeakerManager will fire the onforcespeakerchange because FMRadio stop playing. BTW, once user pause the fmradio, speaker button should be disabled. If User resume the FMRadio and it can directly output to speaker if the speaker status is on. > 10. User presses home button > 11. User launches music app (radio app still is running in the background > but doesn't play radio) > 12. What would speakermanager.speakerforce inside the music app return if > the music app read it? False > 13. User press play in the music app > 14. Where does the music audio come from? Headset > 15. User presses pause in the music app > 16. User switches back to radio app > 17. Does the speaker button still render as pressed? I think if fmradio is disabled, the speaker button should be disabled also. > 18. What does speakermanager.speakerforced inside of the radio app return? > 19. User kills the radio app > 20. User enters music app again. > 21. What would speakermanager.speakerforce inside the music app return if the music app read it? False. (Music AP isn't turn on speaker) > 22. User press play in the music app > 23. Where does the music audio come from? Headset > What would the answers be to questions 12, 14, 17 and 18 be if the user > entered a game which uses the "normal" channel instead of the music app. If there is a FMRaio play in the background with speaker on, and lunch a game, The mixing sound will go through the speaker.

Please land this feature before Friday, 9/20. Failure to do so, will end up being minusing the bug.

It seems unexpected to me that the speaker button doesn't work unless the user first turns on the radio. Likewise it seems unexpected that the speaker turns off if you temporarily pause the radio. Does that really match the UX spec? I would really encourage you to talk through the behavior here with UX. Most sensible thing that I can think of would be something like this interface SpeakerManager : EventTarget { /* query the speaker status */ readonly attribute boolean speakerforced; /* fires when speakerforced changes */ attribute EventHandler onforcespeakerchange; /* force the media stream output to speaker */ attribute boolean forcespeaker; }; If application A sets forcespeaker to true, then speakerforced will be true in all applications as long as application A is visible. If A is switched to the background, then speakerforced switches to false in all applications (unless the new visible application also has set forcespeaker to true). However, if application A starts playing audio while it is visible, then speakerforced remains true for all applications even if application A switches to the background. As soon as application A stops playing audio, speakerforced switches to false again. In other words, speakerforced is only affected by visible applications. Or by an application that that was visible when it started playing audio and it is still playing audio. Does this make sense? Either way you should check with UX. If they are fine with your approach then that's fine too.

hmm, This becomes a UX problem. I will ask UX guys to clarify this behavior. Thanks for feedbacking this problem. 1. If Apps use content audio channel and switch to background, Does we need to switch off the speaker? 2. Would we like this API to be global one? Or Content aware? ie. If now user's device has a background playing music, then user switch to another application and audio isn't under playing. If user press "Speaker On", should we let the audio output from speaker? Or enable it internally until B application start playing, disable it internally while B application stop playing. (But the speaker button still on...)

Hi Preeti, Could you help to find the UX guys to define the speaker button behavior across applications? If application can use this API, we will get the speaker competition problem on platform side.

A quick update - Mike and I met with Randy and SC and have confirmed and clarified the various cases. The cases are covered in the latest version of the spec, which is located at https://mozilla.box.com/s/6yiq70tfptzadbvenade Instead of using box.com's PDF viewer, please download the file to view it. Otherwise the text is illegible. Thanks! - Rob

We would like to let the speaker status concept combine with the audio stream(aka audio-channel) in App. So speaker status cross application wouldn't know each others.

This has been dropped to a P2 from product's perspective and we're past feature complete, which no longer makes this a blocker.

Hi Sicking, Would you like to review it? Or still have another concern?

Comment on attachment 816982 [details] [diff] [review] patch Review of attachment 816982 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/webidl/MozSpeakerManager.webidl @@ +13,5 @@ > + readonly attribute boolean speakerforced; > + /* force the media stream output to speaker */ > + void forcespeaker(boolean enable); > + /* this event will be fired when force speaker status change */ > + attribute EventHandler onforcespeakerchange; It would be good to get more detailed documentation here. Does speakerforced return true only if the current app has called forcespeaker(true)? Or does it return true even when another application forces the speaker to be on? I.e. does speakerforced reflect the phone speaker status, or just application-local data? I think it should reflect the phone status and not application status. Does onforcespeakerchange fire anytime that speakerforced changes? If so we should probably rename it to onspeakerforcedchange to make that more clear (sorry, I think I made the same mistake in a comment in another bug). Does forcespeaker(false) force the speaker to no longer be on? Or does it simply mean that the current application is no longer forcing the speaker to be on? So if another application has called forcespeaker(true) previously, the speaker would still be forced on. What I think the behavior should be (based on the UX spec) is something like this: speakerforced reflects phone status. I.e. if the speaker is currently forced on then this always returns true, no matter which app forced the speaker to be on. onspeakerforcedchange fires anytime speakerforced changes. forcespeaker(true) means that this app attempts to force the speaker to be on. This is only honored if the app is currently in the foreground. So if a foreground app calls forcespeaker(true) that means that 'speakerforced' in all applications switches to true, and any audio from any application will go to the speaker. If a background app calls forcespeaker(true) that doesn't change anything. 'speakerforced' remains false everywhere. If an app that has called forcespeaker(true), but no audio is currently playing in the app itself, is switched to the background we switch 'speakerforced' to false in all apps. If an app that has called forcespeaker(true), and audio is currently playing in the app itself, is switched to the background 'speakerforced' remains true in all apps. If/when the app stops playing audio, 'speakerforced' switches to false in all apps. If an app that has called forcespeaker(true) is switched from the background to the foreground 'speakerforced' switches to true in all apps. I.e. the app doesn't have to call forcespeaker(true) again when it comes into foreground. I *think* this matches what you are proposing, but I wanted to make sure. Can you please verify? If we agree, then I think the API is good except with one minor syntax change. Can we change sm.forcespeaker(true) to sm.forcespeaker = true; The advantage with this is that an application doesn't need to remember separately if it has called forcespeaker(true) or not. It can simply read out whatever status it previously set by doing x = sm.forcespeaker; So sm.forcespeaker returns whether this app is currently attempting to force the speaker on, and sm.speakerforced returns whether the speaker is actually forced on. Does this sound ok?

Nice, thanks for your option and I will improve the document to make things clear. Let me start to write the api wiki page. BTW, We have a request come from RIL team and they want to remove those method from TelephonyProvider attribute bool microphoneMuted; attribute bool speakerEnabled; Could we include the microphone control into this API and change it's name from SpeakerManager to AudioDeviceManager?

So does that mean that you agree with the behavior in comment 66? If so the API looks good to me, though someone else should review the implementation. Let's land what we have now and add microphone stuff separately.

Nominate to v1.3

Agree, Let the SpeakerManager API reflect the device's status. :)

I propose new one for this [Constructor()] interface MozSpeakerManager : EventTarget { /* force the audio output to speaker */ attribute boolean speakerforced; /* this event will be fired when force speaker status change */ attribute EventHandler onspeakerstatuschanged; }; 1 The sm.speakerforced = true, sm would turn on the speaker, else turn off. and UA can get speaker status through sm.speakerforced. So we can remove the sm.forcespeaker(boolean). Does it make sense? 2. onspeakerforcedchange will be fired when phone's speaker status changed. 3. For the competition problem, It seems we should consider the audio stream + visibility in sm, right? Those scenarios are also looking to me.

(In reply to Randy Lin [:rlin] from comment #71) > I propose new one for this > [Constructor()] > interface MozSpeakerManager : EventTarget { > /* force the audio output to speaker */ > attribute boolean speakerforced; > /* this event will be fired when force speaker status change */ > attribute EventHandler onspeakerstatuschanged; > }; > > 1 The sm.speakerforced = true, sm would turn on the speaker, else turn off. That does not seem good. As an application I likely am much more interested in being able to say "I no longer need to force the speaker on" than to say "I'd like to force the speaker off". I.e. as an application author, if I'm no longer interested in forcing the speaker on, I don't want to worry about breaking other applications that are currently forcing the speaker on. > and UA can get speaker status through sm.speakerforced. So we can remove the > sm.forcespeaker(boolean). > Does it make sense? How would this work if you set sm.speakerforced=true while the app is in the background? Throwing an exception seems bad since it would probably break a lot of applications. Exceptions should only be used in exceptional circumstances, and being in the background isn't a very exceptional circumstance. And silently ignoring sm.speakerforced=true also seems bad. It would mean that the application might think that it has turned on the speaker, when in reality it has not. Which could lead to strange behavior when the app is brought to the foreground. > 3. For the competition problem, It seems we should consider the audio stream > + visibility in sm, right? Not sure what you mean here?

(In reply to Jonas Sicking (:sicking) Please ni? if you want feedback from comment #72) > (In reply to Randy Lin [:rlin] from comment #71) > > I propose new one for this > > [Constructor()] > > interface MozSpeakerManager : EventTarget { > > /* force the audio output to speaker */ > > attribute boolean speakerforced; > > /* this event will be fired when force speaker status change */ > > attribute EventHandler onspeakerstatuschanged; > > }; > > > > 1 The sm.speakerforced = true, sm would turn on the speaker, else turn off. > > That does not seem good. As an application I likely am much more interested > in being able to say "I no longer need to force the speaker on" than to say > "I'd like to force the speaker off". I.e. as an application author, if I'm > no longer interested in forcing the speaker on, I don't want to worry about > breaking other applications that are currently forcing the speaker on. > > > and UA can get speaker status through sm.speakerforced. So we can remove the > > sm.forcespeaker(boolean). > > Does it make sense? > > How would this work if you set sm.speakerforced=true while the app is in the > background? Throwing an exception seems bad since it would probably break a > lot of applications. Exceptions should only be used in exceptional > circumstances, and being in the background isn't a very exceptional > circumstance. > > And silently ignoring sm.speakerforced=true also seems bad. It would mean > that the application might think that it has turned on the speaker, when in > reality it has not. Which could lead to strange behavior when the app is > brought to the foreground. > > > 3. For the competition problem, It seems we should consider the audio stream > > + visibility in sm, right? > > Not sure what you mean here? So we want to separate the "ACTION" and "STATUS". UA can do the action(sm.forcespeaker = true), and UA will query through the onspeakerstatuschanged and check the sm.speakerforced attribute. If the application fail to do the sm.forcespeaker=true, should I throw exception or fire onspeakerstatuschanged and let UA to query sm.speakerforced (It should be false) ? Do I get it?

Wiki page created. https://wiki.mozilla.org/WebAPI/SpeakerManager

Looks great. I did some small editorial changes to consistently use the right property names. I do think we could expose this to privileged apps as well since they also have access to fmradio and so likely will also want to enable playing radio through the speaker. We could even expose to unprivileged installed apps, but we can wait with that until it's actually asked for since I don't think there are any strong use cases.

Thanks for review and I am changing my origin patch to fit the behavior that we agree. For the Permissions, hmm, down to Privileged is also ok. Maybe p2p voice communication application need it.

The FMRadio API is also available to privileged apps

(In reply to Randy Lin [:rlin] from comment #76) > Thanks for review and I am changing my origin patch to fit the behavior that > we agree. > For the Permissions, hmm, down to Privileged is also ok. Maybe p2p voice > communication application need it. You mean like a WebRTC video call use case to use speaker phone, right?

Ya, for webRTC, the default audio patch should be in earpiece and user can choose to hear the remote voice through speaker.

Please let's discuss WebRTC audio policies elsewhere. This bug is about the speaker manager API. The fact that the FM radio API is exposed to privileged apps means that we have a use case for exposing speaker manager there too.

(In reply to Jonas Sicking (:sicking) Please ni? if you want feedback from comment #80) > Please let's discuss WebRTC audio policies elsewhere. This bug is about the > speaker manager API. The fact that the FM radio API is exposed to privileged > apps means that we have a use case for exposing speaker manager there too. No, let's not - this plays a role in where you should expose the API. Randy's point is that P2P is a valid use case for using the SpeakerManager API, then that could warrant exposing this to unprivileged applications.

IDL for this API

SpeakerManager API implementation

Others need to change

Comment on attachment 825156 [details] [diff] [review] SpeakerManager P1 Review of attachment 825156 [details] [diff] [review]: ----------------------------------------------------------------- Just a couple of comments, but Andrea should do the real review ::: dom/speakermanager/SpeakerManager.cpp @@ +48,5 @@ > +SpeakerManager::Speakerforced() > +{ > + // If a background app calls forcespeaker=true that doesn't change anything. 'speakerforced' remains false everywhere. > + if (mForcespeaker && !mVisible) { > + return false; If another app sets forcespeaker, and that app is in the forground, then we should still return true here. I.e. we should always reflect the real status of whether the speaker is forced or not. ::: dom/speakermanager/SpeakerManagerService.h @@ +45,5 @@ > + SpeakerManagerService(); > + virtual ~SpeakerManagerService(); > + // Notify to UA if device speaker status changed > + virtual void Notify(); > + void TuruOnSpeaker(bool enable); "TurnOnSpeaker"

Comment on attachment 825154 [details] [diff] [review] hook audiochannel p1 Review of attachment 825154 [details] [diff] [review]: ----------------------------------------------------------------- mIsAudioChannelActive is not used in AudioChannelService and it's never set to false. ::: dom/audiochannel/AudioChannelService.cpp @@ +78,4 @@ > , mPlayableHiddenContentChildID(CONTENT_PROCESS_ID_UNKNOWN) > , mDisabled(false) > , mDefChannelChildID(CONTENT_PROCESS_ID_UNKNOWN) > +, mIsAudioChannelActive(false) You don't use this in AudioChannelService. @@ +284,5 @@ > data->mState = GetStateInternal(data->mType, CONTENT_PROCESS_ID_MAIN, > aElementHidden, oldElementHidden); > + > + if (AnyChannelsActive()) { > + mIsAudioChannelActive = true; Where do you set it to false? ::: dom/audiochannel/AudioChannelServiceChild.cpp @@ +18,5 @@ > #include "nsThreadUtils.h" > > +#ifdef MOZ_WIDGET_GONK > +#include "SpeakerManagerService.h" > +#endif empty line before this and "using.." @@ +81,4 @@ > cc->SendAudioChannelGetState(data->mType, aElementHidden, oldElementHidden, &state); > data->mState = state; > cc->SendAudioChannelChangedNotification(); > + if (AnyChannelsActive()) if (...) { ... } @@ +81,5 @@ > cc->SendAudioChannelGetState(data->mType, aElementHidden, oldElementHidden, &state); > data->mState = state; > cc->SendAudioChannelChangedNotification(); > + if (AnyChannelsActive()) > + mIsAudioChannelActive = true; you never set it to false.

Thanks, This patch is for removing useless variable.

Plus it since it is committed one for v1.3.

Hi baku, Due to this is 1.3 feature, we need your help to review it, thanks a lot.

Comment on attachment 825156 [details] [diff] [review] SpeakerManager P1 Review of attachment 825156 [details] [diff] [review]: ----------------------------------------------------------------- Shutdown() should be call in nsLayoutStatics::Shutdown. r- just because I want to review it again once these comments are applied. I'll promise to be quicker :) ::: dom/speakermanager/SpeakerManager.cpp @@ +11,5 @@ > +#include "nsIDocShell.h" > +#include "nsDOMEvent.h" > +#include "AudioChannelService.h" > + > +using namespace mozilla::dom; Remove this line. @@ +21,5 @@ > + nsIDOMEventListener) > +NS_IMPL_ADDREF_INHERITED(SpeakerManager, nsDOMEventTargetHelper) > +NS_IMPL_RELEASE_INHERITED(SpeakerManager, nsDOMEventTargetHelper) > + > +SpeakerManager::SpeakerManager() : Nit: SpeakerManager::SpeakerManager() : mForcespeaker(false) , mVisible(false) @@ +26,5 @@ > + mForcespeaker(false), > + mVisible(false) > +{ > + SetIsDOMBinding(); > + SpeakerManagerService *service = SpeakerManagerService::GetSpeakerManagerService(); 80 chars @@ +46,5 @@ > + > +bool > +SpeakerManager::Speakerforced() > +{ > + // If a background app calls forcespeaker=true that doesn't change anything. 'speakerforced' remains false everywhere. 80chars @@ +69,5 @@ > + mForcespeaker = aEnable; > +} > + > +void > +SpeakerManager::DispatchSimpleEvent(const nsAString & aStr) nsAString& @@ +170,5 @@ > + nsCOMPtr<nsIDocShell> docshell = do_GetInterface(GetOwner()); > + NS_ENSURE_TRUE(docshell, NS_ERROR_FAILURE); > + docshell->GetIsActive(&mVisible); > + > + // If an app that has called forcespeaker=true is switched from the background to the foreground 'speakerforced' 80> chars ::: dom/speakermanager/SpeakerManager.h @@ +1,5 @@ > +/* This Source Code Form is subject to the terms of the Mozilla Public > + * License, v. 2.0. If a copy of the MPL was not distributed with this file, > + * You can obtain one at http://mozilla.org/MPL/2.0/. */ > + > +#ifndef mozilla_dom_system_SpeakerManager_h mozilla_dom_SpeakerManager_h why _system_ ? @@ +10,5 @@ > + > +namespace mozilla { > +namespace dom { > +/* This class is used for UA to control devices's speaker status. > + * After UA set the speaker status, the UA should handle the forcespeakerchange event and change the speaker status in UI. >80 chars per line. @@ +26,5 @@ > + > +public: > + void Init(nsPIDOMWindow* aWindow); > + > + nsPIDOMWindow* GetParentObject() const I think we should write these methods as: nsPIDOMWindow* GetParentObject() const { ... All of them. @@ +38,5 @@ > + * WebIDL Interface > + */ > + // Get this api's force speaker setting. > + bool Forcespeaker(); > + // Force acoustic sound go through speaker. Don't force to speaker if application stay in the background and reset when application go to foreground 80 chars @@ +39,5 @@ > + */ > + // Get this api's force speaker setting. > + bool Forcespeaker(); > + // Force acoustic sound go through speaker. Don't force to speaker if application stay in the background and reset when application go to foreground > + void SetForcespeaker(bool enable); aEnable @@ +42,5 @@ > + // Force acoustic sound go through speaker. Don't force to speaker if application stay in the background and reset when application go to foreground > + void SetForcespeaker(bool enable); > + // Get the device's speaker forced setting. > + bool Speakerforced(); > + void SetAudioChannelActive(bool isActive); aIsActive @@ +50,5 @@ > + Constructor(const GlobalObject& aGlobal, ErrorResult& aRv); > + > +protected: > + SpeakerManager(); > + virtual ~SpeakerManager(); remove virtual if you use MOZ_FINAL. @@ +51,5 @@ > + > +protected: > + SpeakerManager(); > + virtual ~SpeakerManager(); > + void DispatchSimpleEvent(const nsAString & aStr); nsAString& ::: dom/speakermanager/SpeakerManagerService.cpp @@ +61,5 @@ > + > +NS_IMPL_ISUPPORTS1(SpeakerManagerService, nsIObserver) > + > +void > +SpeakerManagerService::ForceSpeaker(bool enable, uint64_t aChildid) { 1. { new line. 2. sometimes it's childId and sometimes it's childid. @@ +65,5 @@ > +SpeakerManagerService::ForceSpeaker(bool enable, uint64_t aChildid) { > + TuruOnSpeaker(enable); > + > + bool found = false; > + for (uint32_t i = 0; i <mSpeakerStatus.Length(); i++) { Can you use an hashtable for this mSpeakerStatus? ChildID as key. @@ +66,5 @@ > + TuruOnSpeaker(enable); > + > + bool found = false; > + for (uint32_t i = 0; i <mSpeakerStatus.Length(); i++) { > + if (mSpeakerStatus[i].childId == aChildid) { I'm working on a new way to group apps based on window/tab/processID. would be nice to use it here too. I'll file a follow up. @@ +81,5 @@ > + return; > +} > + > +void > +SpeakerManagerService::ForceSpeaker(bool aEnable, bool aVisible) { new line after any { in methods. here and below. @@ +82,5 @@ > +} > + > +void > +SpeakerManagerService::ForceSpeaker(bool aEnable, bool aVisible) { > + // chrome process chrome process, do you mean chrome only code? or b2g main process? @@ +90,5 @@ > + Notify(); > +} > + > +void > +SpeakerManagerService::TuruOnSpeaker(bool on) { 1. aOn 2. TuruOnSpeaker => TurnOnSpeaker ? @@ +103,5 @@ > + audioManager->SetForceForUse(forceuse, nsIAudioManager::FORCE_SPEAKER); > + } else { > + audioManager->SetForceForUse(forceuse, nsIAudioManager::FORCE_NONE); > + } > + return; remove this return. @@ +130,5 @@ > + } > +} > + > +void > +SpeakerManagerService::SetAudioChannelActive(bool aIsAvtive) { aIsActive @@ +197,5 @@ > +SpeakerManagerService::~SpeakerManagerService() > +{ > + MOZ_COUNT_DTOR(SpeakerManagerService); > + AudioChannelService* audioChannelService = AudioChannelService::GetAudioChannelService(); > + if (audioChannelService) if (...) { ... } ::: dom/speakermanager/SpeakerManagerService.h @@ +15,5 @@ > + > +namespace mozilla { > +namespace dom { > + > +class SpeakerManagerService :public on the same line. Or change SpeakerManagerServiceChild.h @@ +27,5 @@ > + virtual void ForceSpeaker(bool aEnable, bool aVisible); > + virtual bool GetSpeakerStatus(); > + virtual void SetAudioChannelActive(bool aIsAvtive); > + // Called by child > + void ForceSpeaker(bool enable, uint64_t aChildid); aEnable @@ +28,5 @@ > + virtual bool GetSpeakerStatus(); > + virtual void SetAudioChannelActive(bool aIsAvtive); > + // Called by child > + void ForceSpeaker(bool enable, uint64_t aChildid); > + // Register for notify the Register for the notification ? Or Register for notify the... what? @@ +29,5 @@ > + virtual void SetAudioChannelActive(bool aIsAvtive); > + // Called by child > + void ForceSpeaker(bool enable, uint64_t aChildid); > + // Register for notify the > + void RegisterSpeakerManager(nsRefPtr<SpeakerManager> aSpeakerManager) { mSpeakerManagers.AppendElement(aSpeakerManager); } 80> chars @@ +30,5 @@ > + // Called by child > + void ForceSpeaker(bool enable, uint64_t aChildid); > + // Register for notify the > + void RegisterSpeakerManager(nsRefPtr<SpeakerManager> aSpeakerManager) { mSpeakerManagers.AppendElement(aSpeakerManager); } > + void UnRegisterSpeakerManager(nsRefPtr<SpeakerManager> aSpeakerManager) { mSpeakerManagers.RemoveElement(aSpeakerManager); } I would like to see an hashtable here. ::: dom/speakermanager/SpeakerManagerServiceChild.cpp @@ +39,5 @@ > + return gSpeakerManagerServiceChild; > +} > + > +void > +SpeakerManagerServiceChild::ForceSpeaker(bool aEnable, bool aVisible) { { in a new line here at below @@ +46,5 @@ > + cc->SendSpeakerManagerForceSpeaker(aEnable && aVisible); > + } > + mVisible = aVisible; > + mOrgSpeakerStatus = aEnable; > + return; remove this. @@ +75,5 @@ > + } > +} > + > +void > +SpeakerManagerServiceChild::SetAudioChannelActive(bool aIsAvtive) { aIsActive @@ +86,5 @@ > +SpeakerManagerServiceChild::SpeakerManagerServiceChild() > +{ > + MOZ_ASSERT(NS_IsMainThread()); > + AudioChannelService* audioChannelService = AudioChannelService::GetAudioChannelService(); > + if (audioChannelService) {} @@ +94,5 @@ > + > +SpeakerManagerServiceChild::~SpeakerManagerServiceChild() > +{ > + AudioChannelService* audioChannelService = AudioChannelService::GetAudioChannelService(); > + if (audioChannelService) {}

Comment on attachment 825157 [details] [diff] [review] misc change p1 Review of attachment 825157 [details] [diff] [review]: ----------------------------------------------------------------- lgtm. ::: dom/ipc/ContentParent.cpp @@ +2719,5 @@ > +{ > +#ifdef MOZ_WIDGET_GONK > + nsRefPtr<SpeakerManagerService> service = > + SpeakerManagerService::GetSpeakerManagerService(); > + *aValue = false; aValue should be set to false if MOZ_WIDGET_GONK is not initialized. ::: dom/ipc/PContent.ipdl @@ +260,3 @@ > async DumpMemoryInfoToTempDir(nsString identifier, > bool minimizeMemoryUsage, > bool dumpChildProcesses); why do you remove this line? ::: layout/build/nsLayoutStatics.cpp @@ +407,4 @@ > nsDocument::XPCOMShutdown(); > > CacheObserver::Shutdown(); > + remove this line.

Thanks baku's commnet and I learn a lot, I will provide new one asap. :)

Change the return value in IPC and include the webidl in this patch.

Prefer to store the audiomanagerService object via nsTArray. To enumerate the items in the hashtable seems need more code to do the notify.

1.Use CheapSet to store the child Speaker status. 2.Fix a lot of nits, thanks correction of this part.

Randy, Will this one to be landed on time before 11/22?

It may depend on the review process. Hi Baku, Got problems?

Hi Jonas, Would you kindly suggest another reviewer? It look like amarchesini is busy...

Possibly someone on roc's team. Other than that I don't know who else have worked on this code.

Comment on attachment 829736 [details] [diff] [review] hook audiochannel p3 Review of attachment 829736 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/audiochannel/AudioChannelService.cpp @@ +171,5 @@ > UnregisterType(data->mType, data->mElementHidden, > CONTENT_PROCESS_ID_MAIN, data->mWithVideo); > } > +#ifdef MOZ_WIDGET_GONK > + for (uint32_t i = 0; i < mSpeakerManager.Length(); i++) { What about if AnyAudioChannelIsActive() is not called at any iteration? bool active = AnyAudioChannelIsActive(); for (...) ::: dom/audiochannel/AudioChannelServiceChild.cpp @@ +125,5 @@ > obs->NotifyObservers(nullptr, "audio-channel-agent-changed", nullptr); > } > +#ifdef MOZ_WIDGET_GONK > + for (uint32_t i = 0; i < mSpeakerManager.Length(); i++) { > + mSpeakerManager[i]->SetAudioChannelActive(AnyAudioChannelIsActive()); ditto

Comment on attachment 829737 [details] [diff] [review] SpeakerManager implement p2 Review of attachment 829737 [details] [diff] [review]: ----------------------------------------------------------------- lgtm. Can you provide some mochitests? ::: dom/speakermanager/SpeakerManager.cpp @@ +24,5 @@ > + , mVisible(false) > +{ > + SetIsDOMBinding(); > + SpeakerManagerService *service = > + SpeakerManagerService::GetSpeakerManagerService(); if (service) .. @@ +30,5 @@ > +} > + > +SpeakerManager::~SpeakerManager() > +{ > + SpeakerManagerService *service = SpeakerManagerService::GetSpeakerManagerService(); if (service) @@ +49,5 @@ > + // 'speakerforced' remains false everywhere. > + if (mForcespeaker && !mVisible) { > + return false; > + } > + SpeakerManagerService *service = SpeakerManagerService::GetSpeakerManagerService(); ditto @@ +62,5 @@ > + > +void > +SpeakerManager::SetForcespeaker(bool aEnable) > +{ > + SpeakerManagerService *service = SpeakerManagerService::GetSpeakerManagerService(); ditto @@ +188,5 @@ > + // in the app itself, if application switch to the background, we switch 'speakerforced' > + // to false. > + if (!mVisible) { > + AudioChannelService* audioChannelService = AudioChannelService::GetAudioChannelService(); > + if (mForcespeaker && audioChannelService && !audioChannelService->AnyAudioChannelIsActive()) { 80chars ? @@ +199,5 @@ > +void > +SpeakerManager::SetAudioChannelActive(bool isActive) > +{ > + if (!isActive && !mVisible) { > + SpeakerManagerService *service = SpeakerManagerService::GetSpeakerManagerService(); ditto

ah... and sorry for the delay. I was away last week.

Thanks for your help, I will add test case later.

Comment on attachment 8336534 [details] [diff] [review] test case for MozSpeakerManager. Review of attachment 8336534 [details] [diff] [review]: ----------------------------------------------------------------- is it fully green on try? ::: dom/speakermanager/tests/moz.build @@ +3,5 @@ > +# This Source Code Form is subject to the terms of the Mozilla Public > +# License, v. 2.0. If a copy of the MPL was not distributed with this > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +MOCHITEST_MANIFESTS += ['mochitest.ini'] Do this: 1. add MOCHITEST_MANIFESTS += ['tests/mochitest.ini'] in ../moz.build 2. remove this moz.build it should work. ::: dom/speakermanager/tests/test_speakermanager.html @@ +16,5 @@ > + > + function testObject() { > + var mgr = new MozSpeakerManager(); > + var spkforced = false; > + mgr.onspeakerforcedchange = function () { extra space before () @@ +41,5 @@ > + SimpleTest.finish(); > + } > + } > + > + SimpleTest.expectAssertions(0, 9); why? @@ +43,5 @@ > + } > + > + SimpleTest.expectAssertions(0, 9); > + SimpleTest.waitForExplicitFinish(); > + if (SpecialPowers.hasPermission("speaker-control", document)) { SpecialPowers.pushPermissions( [{ "type": "speaker-control", "allow": 1, "context": document }], startTests); and remove the reload.

Sorry I just want to let it r- because I found it cause some problem and debugging. I will send another one and let try to be green. :)

Comment on attachment 8336534 [details] [diff] [review] test case for MozSpeakerManager. r-, still have yellow fail.

Hi Baku, Here is the test case for this API. run result can pass, the build config also modified. thanks https://tbpl.mozilla.org/php/getParsedLog.php?id=30955775&tree=Try 05:22:18 INFO - 13947 INFO TEST-START | /tests/dom/speakermanager/tests/test_speakermanager.html 05:22:18 INFO - 13948 INFO TEST-PASS | /tests/dom/speakermanager/tests/test_speakermanager.html | speaker should be true 05:22:18 INFO - 13949 INFO TEST-PASS | /tests/dom/speakermanager/tests/test_speakermanager.html | speaker should be false 05:22:18 INFO - 13950 INFO TEST-INFO | MEMORY STAT vsize after test: 103469056 05:22:18 INFO - 13951 INFO TEST-INFO | MEMORY STAT residentFast after test: 51265536 05:22:18 INFO - 13952 INFO TEST-INFO | MEMORY STAT heapAllocated after test: 21885960 Randy

Fix nits, include test case.

Comment on attachment 8336926 [details] [diff] [review] test.patch Review of attachment 8336926 [details] [diff] [review]: ----------------------------------------------------------------- I meant to keep mochitest.ini in the tests folder and write this in the moz.build: MOCHITEST_MANIFESTS += ['tests/mochitest.ini'] ::: dom/speakermanager/moz.build @@ +3,5 @@ > +# This Source Code Form is subject to the terms of the Mozilla Public > +# License, v. 2.0. If a copy of the MPL was not distributed with this > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +MOCHITEST_MANIFESTS += ['mochitest.ini'] this should be tests/mochitest.ini

Comment on attachment 8336926 [details] [diff] [review] test.patch Review of attachment 8336926 [details] [diff] [review]: ----------------------------------------------------------------- wait... why moz.build is part of this patch? should it be part of previous patches?

Comment on attachment 8336933 [details] [diff] [review] check-in patch Review of attachment 8336933 [details] [diff] [review]: ----------------------------------------------------------------- ::: dom/ipc/ContentParent.cpp @@ +2655,5 @@ > +{ > +#ifdef MOZ_WIDGET_GONK > + nsRefPtr<SpeakerManagerService> service = > + SpeakerManagerService::GetSpeakerManagerService(); > + *aValue = false; Nit: move this line before nsRefPtr< ... ::: dom/speakermanager/SpeakerManager.cpp @@ +195,5 @@ > + } > + // If an application that has called forcespeaker=true, but no audio is > + // currently playing in the app itself, if application switch to > + // the background, we switch 'speakerforced' to false. > + if (!mVisible) { Nit: if (!mVisible && mForcespeaker) { AudioChannelService* audioChannelService = AudioChannelService::GetAudioChannelService(); if (audioChannelService && !audioChannelService->AnyAudioChannelIsActive()) { service->ForceSpeaker(false, mVisible); } } ::: dom/speakermanager/SpeakerManagerService.cpp @@ +14,5 @@ > +#include "nsIPropertyBag2.h" > +#include "nsThreadUtils.h" > +#include "nsServiceManagerUtils.h" > +#include "AudioChannelService.h" > +#include <cutils/properties.h> new line after the last #include @@ +137,5 @@ > + } > +} > + > +NS_IMETHODIMP > +SpeakerManagerService::Observe(nsISupports* aSubject, const char* aTopic, const PRUnichar* aData) 80 chars? ::: dom/speakermanager/mochitest.ini @@ +1,2 @@ > +[DEFAULT] > + move this mochitest.ini into tests folder ::: dom/speakermanager/moz.build @@ +3,5 @@ > +# This Source Code Form is subject to the terms of the Mozilla Public > +# License, v. 2.0. If a copy of the MPL was not distributed with this > +# file, You can obtain one at http://mozilla.org/MPL/2.0/. > + > +MOCHITEST_MANIFESTS += ['mochitest.ini'] tests/mochitest.ini

Fix nits, try result https://tbpl.mozilla.org/?tree=Try&rev=939e31bbefb5 check-in needed..

Per discussion offline - speaker manager API is a targeted feature, not committed, which means we don't need to block on this. We should try to get this in for 1.3 still, however.

We targeted this feature in 1.2 and decided to push this to 1.3. From product perspective this is an important feature for our partners. We should definitely try to get this into 1.3

I am clearing the outdated ni? for myself as it sounds like Sri is adding this to the backlog for 1.3, and that the feature will be designed and addressed as part of that effort.

What's the latest status here for 1.3? Thanks.

(In reply to Chris Lee [:clee] from comment #120) > What's the latest status here for 1.3? Thanks. This is already landed for 1.3.

UX please take a look

Preeti, what is the UX need here? I can't determine the issue from the lengthy thread - sorry. :(

Initial test pass is finished here - mostly looks okay overall with a couple of followups - see the dependencies.

Clearing ni? as this is Verified Fixed + see above.