Closed Bug 1712892 Opened 3 years ago Closed 1 year ago

Support permanent permissions for selectAudioOutput({deviceId})

Categories

(Firefox :: Site Permissions, enhancement, P1)

enhancement

Tracking

()

RESOLVED FIXED
110 Branch
Tracking Status
firefox110 --- fixed

People

(Reporter: karlt, Assigned: karlt)

References

()

Details

Attachments

(5 files)

This is about changes to WebRTCParent.jsm so that selectAudioOutput({deviceId}) can sometimes return without prompting and to SitePermissions.jsm gPermissions._permissions for pageinfo/permissions.js.

UI for selectAudioOutput() involves both permission and a choice of speakers. This may present a difficultly for a "Remember this decision" checkbox label if there is a lack of clarity over whether the permission or choice or both is/are remembered. Perhaps this can be improved with a better label name. The API is designed so that content can remember the choice of speakers if permission has been granted permanently, so it shouldn't be necessary for the browser to remember the choice of speakers, unless to prevent prompt-spamming.

The "Always Ask" and "Block" permission states can fit well with the combined permission/choice model, but the design around "Allow" is less clear.
There is a choice to make re whether to implement "Allow" either per speaker device or for all speaker devices. If permission is granted for all speaker devices the UI may need to clarify that this is happening. (Compare bug 1142123 comment 5.)

Even if "Allow" is for all speakers, the user will need to have chosen any speaker at some point in the past (perhaps on another same-site page previously visited) for content to be able to specify the deviceID. There is a choice to make re whether device IDs provided prior to an all-speaker grant should be rotated so that permission is not unintentionally granted to these also.

There is an option to consider of making permission grants always permanent. With this option, the prompt might have "OK", "Cancel", and "Always Block" buttons, instead of a remember checkbox. The Permission state set might contain "Blocked", "Ask for all non-default speakers", and "Some non-default speakers are allowed".

When selectAudioOutput() is called without a deviceId parameter, there is always a choice of speakers to present to the user. At least in the case when permission has already been granted for a device, the UI would preferably feel like making a choice of speakers rather another request for the same permission.

There may be options to integrate even default speaker permission into the same model. The default speaker would be allowed by default, but perhaps could be blocked or muted.

See Also: → 1712894
See Also: → 1712895
See Also: → 1712898

Karl, could you help assign a priority? Do we target a specific Firefox version with this?

Severity: -- → N/A
Flags: needinfo?(karlt)

(In reply to Karl Tomlinson (:karlt) from comment #0)

There is a choice to make re whether to implement "Allow" either per speaker device or for all speaker devices.

My understanding is this is inherently per speaker, because this is first and foremoest a device picker, not a permission prompt. It's meant to stop JS from building its own in-content dropdown list, and instead call this one directly from the button showing the current selection: click → Speakers: System default which calls:

button.onclick = async () => {
  if (!deviceId) {
    deviceId = await navigator.mediaDevices.selectAudioOutput();
    button.innerText = (await navigator.mediaDevices.enumerateDevices()).filter(d => d.deviceId == deviceId).label;
  }
  await video.setSinkId(deviceId);
}

...which then reads: Speakers: Jan-Ivar's AirPods

be called by JS in place of implementing in-content

If permission is granted for all speaker devices the UI may need to clarify that this is happening.

We wouldn't do that, because that would defeat getting rid of in-content device selection.

Instead, my design on this was that checking ☑ Remember this decision would control whether we persist deviceIds to local storage. This lets JS bypass the prompt on future visits like this:

button.onclick = async () => {
  if (!deviceId) {
    deviceId = localStorage.speakersId;
    deviceId = await navigator.mediaDevices.selectAudioOutput({deviceId}); // may bypass prompt
    button.innerText = (await navigator.mediaDevices.enumerateDevices()).filter(d => d.deviceId == deviceId).label;
    localStorage.speakersId = deviceId;
  }
  await video.setSinkId(deviceId);
}

It's important that JS still go through the selectAudioOutput method, so we can prompt if the device is no longer permitted, providing a disincentive for trackers to validate these ids with setSinkId to fingerprint people.

In spite of the JS model being quite different, this should give a familiar user experience in that clicking ☑ Remember this decision should side-step any future prompts from sites that want to reuse the speakers used last time.

Karl, does this match your understanding? With this model in mind, I had some difficulty parsing some of the suggestions in comment 0.

Attached image SelectSpeakers.png

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #2)

... this is first and foremost a device picker, not a permission prompt.

This is also why it's critical that the UX be updated to remove the drop-down within the drop-down (non-accurate idea mockup).

  • With getUserMedia, most users will want the default (device picking is secondary)
  • With selectAudioOutput invoked from a System default button, users are there to pick (device picking is the primary task)

Also, this being a picker, our temporary block model isn't always appropriate. But this is an existing issue with getDisplayMedia and camera/mic switching. See bug 1609578 and bug 1695757.

No target release at this stage, but I'd like to start the decision making process for determining a UX.

Flags: needinfo?(karlt)
Priority: -- → P1

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #3)

This is also why it's critical that the UX be updated to remove the drop-down within the drop-down (non-accurate idea mockup).

Ah, a multi-line select instead of a single-line, yes.

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #2)

With this model in mind, I had some difficulty parsing some of the suggestions in comment 0.

The "permission grants always permanent" paragraph essentially involves dropping the "Remember"/"Always" checkbox and always behaving like this is checked.

Granting permanent access to another speaker does not risk disclosing private information like permanent access to a microphone or camera would. (The speaker device info label has already been disclosed to the site with a temporary grant.)

Yes, a situation where a temporary grant is preferred is conceivable, but would it be worthwhile making the permanent grant process more difficult to support a temporary grant option, or can we just focus on the permanent grant case?
That would let us make a cleaner device picker.

The exposure of the deviceID would then be the permission grant. When revoked (by specific UI or by "Clear cookies and site data") , we refresh all deviceIDs.

I agree focusing on the permanent grant case makes sense. We already have the 3-choice Allow, Block/Always Block UX construct in screen-sharing.

... always behaving like this is checked.

...for the granted state. For the not-granted state, I think we still need to figure out what we want (comment 4). I assume names of actions need to be consistent with our other dialogs, so perhaps we can pin behavior to labels already, to simplify discussion:

  • "Cancel" - Dismiss dialog without a temporary block.
  • "Block" - Dismiss dialog with a temporary block.
  • "Always Block" - Dismiss dialog with a persistent block.

(In reply to Jan-Ivar Bruaroey [:jib] (needinfo? me) from comment #8)

For the not-granted state, I think we still need to figure out what we want (comment 4).

  • "Cancel" - Dismiss dialog without a temporary block.
  • "Block" - Dismiss dialog with a temporary block.
  • "Always Block" - Dismiss dialog with a persistent block.

I see clear use cases for "Cancel" and "Always Block":

  • "Cancel" for backing out of a process to return to previous state.
  • "Always Block" for sites that maliciously prompt on every user action or for users that prefer to use OS output-device selection.

I see much less value in the temporary "Block". "Cancel" is sufficient to ensure no sound will be delivered from other speakers and sites should not be sending further prompts unless somehow user-initiated. For sites that prompt when not intended by the user, the "Always Block" option is available. I still expect to provide a mechanism to change previous decisions.

"desktop-notification" now provides "Always Block" and always "Allow", with no temporary options. A key difference between selectAudioOutput() and Notification.requestPermission() is that selectAudioOutput() is an action that a user is likely to trigger but then cancel without intending to block future actions (temporarily or persistently).

See Also: → 1726728
Assignee: nobody → karlt
See Also: → 1800286
Blocks: 1800287
Depends on: 1800580

A global "Allow" option is not provided because the permission grant prompt is
primarily a device selection, which will (with subsequent patches) grant
permanent permission for the selected device.

Bug 1800287 covers setting a similar block from the selectAudioOutput() prompt.

Depends on D162078

so as to add support for double-keyed per device permissions, which can then
all be cleared from a single item/button.

Depends on D162232

Permission grants are permanent, supporting content use of
selectAudioOutput(previousDeviceId) to maintain the same output(s) on
subsequent same-origin page loads.

Permissions can be revoked from the site permissions panel.

Depends on D162233

Blocks: 1804352

https://phabricator.services.mozilla.com/D162232#5334946 raised a concern about content continuing to present the speaker chooser when transient activation is available if the default deny option is "Cancel"/"Not Now" instead of "Block (for the lifetime of this Window)".

This testcase compares different UI that can be presented by content on user gesture.
Set pref "media.setsinkid.enabled" for the speaker selection.

The HTML select is in some ways more obstructive than the "webRTC-shareDevices" PopupNotifications panel because it grabs mouse input, but it is easier to dismiss. The popup windows are harder to dismiss.

Also relevant is the motivation content might have to persistently present the UI until the user gives in and says yes. With selectAudioOutput(), all that content stands to gain is audio output to (and device name of) one set of speakers chosen by the user. Before the user says yes, content already has access to audio output to the default set of speakers.

Attachment #9303754 - Attachment description: Bug 1712892 add Page Info UI to block speaker selection r?pbz → Bug 1712892 add Page-Info Permissions-tab item to block speaker selection r?pbz
Pushed by ktomlinson@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/a6143d316f28
add Page-Info Permissions-tab item to block speaker selection r=pbz
https://hg.mozilla.org/integration/autoland/rev/224c77c864c2
use _createWebRTCPermissionItem() for speaker selection permissions r=pbz
https://hg.mozilla.org/integration/autoland/rev/fe5ad2ea1077
skip selectAudioOutput() prompt if the requested device has been previously allowed r=jib
Duplicate of this bug: 1712894
Duplicate of this bug: 1712895
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: