Desktop permissions prompts remember what you chose last time. We should too. We could do that per-site or just a global persistence...

This stores a per-host pref, "media.webrtc.videoSource.default.org.example.com" for example, that holds the non-localized "name" of the camera (for example "Camera front, Rotate 270") If that isn't specified, it falls back to "media.webrtc.videoSource.default". The list is reordered so that will be first (and show up as default). Maybe we should set that second default to whatever was last used so that, if you always use the Front camera, we'll default to that in most cases...

How does desktop persist the state? I tried looking for a bit but I don't think I saw their approach.

I think I was wrong. Desktop doesn't remember. Desktop does recycle their menulists: http://mxr.mozilla.org/mozilla-central/source/browser/modules/webrtcUI.jsm#358 so those retain state within a particular session.

(In reply to Wesley Johnston (:wesj) from comment #3) > I think I was wrong. Desktop doesn't remember. Desktop does recycle their > menulists: > > http://mxr.mozilla.org/mozilla-central/source/browser/modules/webrtcUI. > jsm#358 > > so those retain state within a particular session. If desktop doesn't remember, how important is it for mobile to remember? (In reply to Wesley Johnston (:wesj) from comment #1) > This stores a per-host pref, > "media.webrtc.videoSource.default.org.example.com" for example, that holds > the non-localized "name" of the camera (for example "Camera front, Rotate I think some of my worry about this patch comes from my dislike for the way we are storing the preference. Is this the typical way we name pre-host preferences? Shouldn't we just use nsIContentPrefService?

Comment on attachment 8448488 [details] [diff] [review] Patch v1 >diff --git a/mobile/android/chrome/content/WebrtcUI.js b/mobile/android/chrome/content/WebrtcUI.js >- _addDevicesToOptions: function(aDevices, aType, aOptions, extraOptions) { >- if (aDevices.length) { >+ _addaDevicesToOptions: function(aDevices, aType, aOptions, aHost) { _addaDevicesToOptions? I think this is a typo >+ setDefaultDevice: function(aType, value, aHost) { >+ getDefaultDevice: function(aType, aHost) { I'd like to figure out if nsIContentPrefService2 is better suited. >+ // Bsaed on the aTypes available, setup the prompt and icon text typo: Based > let host = aContentWindow.document.documentURIObject.host; >+ // Show the app name if this is a WebRT app, otherwise show the host. > let requestor = BrowserApp.manifest ? "'" + BrowserApp.manifest.name + "'" : host; Put the comment above the "let host" line or add a blank line before the comment >+ this._addaDevicesToOptions(videoDevices, this.VIDEO_SOURCE, options, host); >+ this._addaDevicesToOptions(audioDevices, this.AUDIO_SOURCE, options, host); _addaDevicesToOptions fixup This patch makes the list of items order in the dropdown no longer be consistent across websites. That might be a good thing or a bad thing. I think we should get UX to be aware of this potential change too.

Anthony/Yuan: Just wanted any feedback you had for this idea. We can show a screenshot of the affected UI and how it would change if we want to land this idea.

(In reply to Mark Finkle (:mfinkle) from comment #4) > > so those retain state within a particular session. > > If desktop doesn't remember, how important is it for mobile to remember? If you want some UX-y answer, we've done plenty of things try to reduce taps for people on mobile. This would help. If you want my answer, the chattyness and forgetfulness of Desktop's notifications/permissino prompts is awful. I don't think its a good model to follow. > (In reply to Wesley Johnston (:wesj) from comment #1) > I think some of my worry about this patch comes from my dislike for the way > we are storing the preference. Is this the typical way we name pre-host > preferences? Shouldn't we just use nsIContentPrefService? I've never heard of nsIContentPrefService. Looks like a good fix.

Uses the content pref service. Since the service is async, moved this over to use some promises.

Comment on attachment 8485935 [details] [diff] [review] Patch v2 >+ // Bsaed on the aTypes available, setup the prompt and icon text > let requestType; Typo: // Based >+ let loadContext = aWindow.QueryInterface(Ci.nsIInterfaceRequestor) >+ .getInterface(Ci.nsIWebNavigation) >+ .QueryInterface(Ci.nsILoadContext); >+ // videoSource is both the string used for l10n lookup and the object that will be returned >+ this._addDevicesToOptions(videoDevices, this.VIDEO_SOURCE, options, host, loadContext).then((aOptions) => { >+ return this._addDevicesToOptions(audioDevices, this.AUDIO_SOURCE, aOptions, host, loadContext); >+ }).catch(Cu.reportError).then((aOptions) => { >+ let buttons = this.getDeviceButtons(audioDevices, videoDevices, aCallID, host); >+ NativeWindow.doorhanger.show(message, "webrtc-request", buttons, BrowserApp.selectedTab.id, aOptions); >+ }); And they say Promises make code less complex and more easy to read... Do some manual testing to make sure this doesn't break. I think the is a test for this as well. Do a Try run. r+ but FIX the typo!

(In reply to Mark Finkle (:mfinkle) from comment #6) > Anthony/Yuan: Just wanted any feedback you had for this idea. We can show a > screenshot of the affected UI and how it would change if we want to land > this idea. yes please! I see what we're trying to do here but just to be clearer, screenshots would help. On first thought, I don't see why we shouldn't remember preferences or at least have an option? But I could be mistaken, will need to see this first, having trouble reproducing/triggering it on my end.

Jib, does this interact correctly with constraints? Like your facingMode work? If the site requests a specific facingMode IIRC we change the default.

Comment on attachment 8485935 [details] [diff] [review] Patch v2 (In reply to Gian-Carlo Pascutto [:gcp] from comment #12) > Jib, does this interact correctly with constraints? No this conflicts with our implementation of constraints. Our interpretation of the spec is that optional constraints coming from a webpage are truly optional, so we give the user the option to override them. For example: On a webpage specifying { video: { facingMode: "user" } } as optional, the end-user gets a choice between front and back camera, with front chosen as the default. [1] This lets our users pick the back camera even though the webpage has stated it *prefers* the front camera (the webpage has told us it will work with back cameras after all; some devices may not have a front camera). Now that doesn't mean that me, as a user of the webpage, picking the back camera was a good idea, so and I'm not sure I'd want my phone to remember my bad choice for next time over the webpage's preference. If there were a direct conflict between user preference and webpage preference, I'd tend to lean toward the user, except in this case, we're merely inferring user preference from what they used last time, which seems insufficient to trump webpage preference to me. While I'm not averse to us trying to remember a user's default, it's not entirely clear to me how this should work in light of constraints. For instance, if the webpage has changed its constraints from last time, does the user's choice from last time still trump what would have been the default otherwise? --- [1] This works because the list of devices returned to WebrtcUI.js is pre-filtered (by any required constraints) and pre-sorted (by any optional constraints), and because WebrtcUI.js uses the first item on the list as the default. This patch would break that.

I think this should be backed out.

I think what's missing here is a way for the patch to learn whether the "last-used" value is on the webpage's "preferred list", and have it be a no-op otherwise. Since constraints can be rather complicated, this may require a change in mozGetUserMediaDevices.

Consider this test page which is now effectively broken by this patch: http://mozilla.github.io/webrtc-landing/pc_test_swap.html It lets you swap between front and back cameras on Fennec by clicking a [Swap video] button on the page (on desktop it merely flashes to black and back), and uses our new replaceTrack API to alter what's sent over a peerConnection. Yeah, it sucks because you get a doorhanger every time you click it, but importantly, the first time you click the button the doorhanger would ask you for "Back camera", and the next time you click it the doorhanger would ask you for your "Front camera" (the button's true toggle nature becomes clearer once persistent permissions are granted). With this patch, the web-page's intent to switch camera is effectively ignored.

backed out: https://hg.mozilla.org/integration/fx-team/rev/a3ae1a0cf65f

Merge of backout: https://hg.mozilla.org/mozilla-central/rev/a3ae1a0cf65f

If webrtc is relying on the ordering of arguments they hand to us rather than just telling us what the user asked for, I don't think there's much we can do here. I'm flipping this bug to them.

jib: so, remembering the user's last choice Our options: 1) site gives no indication Default to last-used - could be per-site, but that might be confusing more than it help. 2) Site gives optional which-camera indication a) ignore last-used and don't update it, or b) ignore last-used unless the user had overridden previously *for this site* (and maybe as well *for this preference of camera* (see below about GetUserMediaDevices)) c) ignore site preferences entirely and use last-used. I dislike this a lot. Note also once we implement GetUserMediaDevices for applications they may say "give me this camera" where they've either remembered the id or figured out via other means which they want as a default. This would apply especially to multi-camera conferencing setups. This would mesh well with 2a, and with 2b *if* we also key the lookup on what device is being asked for. I would suggest we use 1 (global, not per-site), and either 2a (simple) or 2b (probably a bit painful, and arguable how much a gain it is). Probably 2a.

I think that once we finish constraints then this problem will largely go away as adoption sets in. Sites will have the tools to pick suitable first-time defaults as well as remember last-used or preferred settings for the user for the *specific task at hand* (of which there may be several). For instance, a travel webapp may have both a WebRTC-powered "concierge" feature (default front) as well as a "scan receipt" feature (default back). The browser-notion of "last-used" is inadequate here, as it assumes a single purpose. So priority-wise I would put this behind finishing constraints and mediaDevices. "Unconstrained" isn't a special-case. Any number of constraints may produce any number of cameras, both preferred and non-preferred. deviceId is just another constraint. If we still want to pursue this then I would state the logic like this: Ignoring site-preferences would be a spec violation, so any tilting toward "last-used" would need to be strictly between "equally site-preferred" choices. If we do that then there are no conflicts, and it still works as you'd expect in simple WebRTC demos.

To make this happen, the internal mozGetUserMediaDevices must add a way to communicate out which of the devices in the list are preferred vs. non-preferred. The patch could then be modified to make any defaulting to last-used only happen when a) the last-used is on the preferred list or b) there are no preferred items on the returned list.

I said this on IRC, but after using Hello a few times I'm finding this a real dogfood annoyance. I use a USB headset, so every time I join a call I have to pick my headset mic from the dropdown. Constraints seem great, but if we're relying on every single getUserMedia-using application to get this right that seems like a stretch. Even if this only worked in the "no constraints" case that would seem like a win to me, since we'd get more useful behavior for the default case.

This seems doable in the simple zero-constraints case. Ted is on Windows, so to those on OSX (unless you have some custom USB thing) beware you're benefitting OSX's automatic headset-hotplugging feature that's exclusive to its built-in audio device only, so the UX story is worse on other platforms. As abr mentioned, we'd likely want this "last used" memory per-origin, as this fits well with persistent permissions (see bug 1145525)

Mass change P3->P4 to align with new Mozilla triage process.