Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Distinguish cancellations from absent OS permissions #281

Open
eladalon1983 opened this issue Sep 21, 2023 · 30 comments
Open

Distinguish cancellations from absent OS permissions #281

eladalon1983 opened this issue Sep 21, 2023 · 30 comments

Comments

@eladalon1983
Copy link
Member

Video conferencing apps run into a common issue - the app offers the user the ability to share their screen, but when the user tries to avail oneself of the option, it turns out that the browser is lacking the necessary OS permissions.

Worse, the app doesn't have a standard way of distinguishing missing OS permissions from a normal cancellation. In practice, both Chrome and Firefox set the message to "Permission denied by system". But that's not specified.

In the short term, I suggest that the Permission Failure step in the gDM algorithm be modified to set either a different name, message or code for failures stemming from lacking OS permissions.

In the mid term, I suggest that we think of something that could allow the app to query and see if the browser even has the prerequisite permissions. (Concerns of fingerprinting could be mitigated by gating this on mic/camera permissions.)

In the long term, one might argue that everyone should move to a model like Apple's macOS-based picker. I would object on the grounds that OS admin policies might still block such APIs.

@eladalon1983 eladalon1983 changed the title Distinguish cancellations from absent-permissions Distinguish cancellations from absent OS permissions Sep 21, 2023
@eladalon1983
Copy link
Member Author

(I imagine message is not a good candidate for this due to localization concerns, but it should be mentioned that this is what video conferencing tools are currently using.)

@beaufortfrancois
Copy link
Contributor

Looking at https://wicg.github.io/serial/, it seems like you could also create your own DOMException like "MissingOperatingSystemPermissionError" for instance. This is done for "BreakError","FramingError", and "ParityError" in this spec.

https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/renderer/platform/bindings/exception_code.h;l=107;drc=b74163d3d63e781ce9899ec6a4e5a7c113d960b9 suggests they are more specs defining their own DOMExceptions.

@eladalon1983
Copy link
Member Author

Using MissingOperatingSystemPermissionError as the name works for me. But where do you see that in https://wicg.github.io/serial/, @beaufortfrancois?

@chrisguttandin
Copy link

Not sure if it helps but there is an OverconstrainedError in Media Capture and Streams.

@beaufortfrancois
Copy link
Contributor

beaufortfrancois commented Sep 21, 2023

Using MissingOperatingSystemPermissionError as the name works for me. But where do you see that in https://wicg.github.io/serial/, @beaufortfrancois?

It is not in the Web Serial API. The "MissingOperatingSystemPermissionError" name as a suggestion only.

@eladalon1983
Copy link
Member Author

Gotcha. Works for me.

@youennf
Copy link
Collaborator

youennf commented Sep 21, 2023

Keeping NotAllowedError seems like a good idea to me, for backward compatibility and for any website that just displays a generic message about denied permission.
Adding more standard information (message or something else) in case of NotAllowedError might be ok.

In general, I think the UA is ideally suited for helping users understand what is going on, whether they should care about changing permissions, and if so how to do it.

Also, this does not seem specific to getDisplayMedia. IIRC, similar discussions happened for getUserMedia.

@eladalon1983
Copy link
Member Author

Adding more standard information (message or something else) in case of NotAllowedError might be ok.

Do we know of precedents where the message's contents are specified?

In general, I think the UA is ideally suited for helping users understand what is going on, whether they should care about changing permissions, and if so how to do it.

You'd think so, but in practice, no browser I know of does that. Realistically, when can we expect to get to it? Until we do, Web applications are trying to fill in the gap; helping them do so easily and interoperably should be easy for us.

Also, this does not seem specific to getDisplayMedia. IIRC, similar discussions happened for getUserMedia.

Agreed.

@eladalon1983
Copy link
Member Author

eladalon1983 commented Sep 28, 2023

Concrete proposal - following the example set by RTCError, we extend DOMException as follows:

enum GetDisplayMediaNotAllowedReason {
  "user-rejected",
  "operating-system-disallowed",
};

dictionary GetDisplayMediaNotAllowedErrorInit {
  required GetDisplayMediaNotAllowedReason reason;
};

interface GetDisplayMediaNotAllowedError : DOMException {
  constructor(optional DOMString message = "", GetDisplayMediaNotAllowedErrorInit init);
  readonly attribute GetDisplayMediaNotAllowedReason reason;
};

The ctor sets the name attribute to the value NotAllowedError.

Wdyt? (@alvestrand, @youennf, @jan-ivar)

@youennf
Copy link
Collaborator

youennf commented Sep 28, 2023

The ctor sets the name attribute to the value InvalidStateError.

NotAllowedError?

@alvestrand
Copy link
Contributor

Coincidentally, there was a pointer passed by today about guidelines for extending DOMError with extra info:

https://webidl.spec.whatwg.org/#idl-DOMException-derived-interfaces

@eladalon1983
Copy link
Member Author

NotAllowedError

Thanks. Made the correction.

Coincidentally, there was a pointer passed by today about guidelines for extending DOMError with extra info:

https://webidl.spec.whatwg.org/#idl-DOMException-derived-interfaces

Thanks for sharing.
Having the first parameter as optional and the second one as non-optional is quite odd to my C++ sensibilities, but I've gone ahead and adopted that in my concrete proposal.

@youennf
Copy link
Collaborator

youennf commented Sep 29, 2023

Having the first parameter as optional and the second one as non-optional is quite odd to my C++ sensibilities

I am curious how it would work when calling new GetDisplayMediaNotAllowedError(). Which reason do you get?
I would tend to try making the second parameter optional if we can.

@eladalon1983
Copy link
Member Author

My reading here is that this is recommending a non-optional dictionary. But I guess the dictionary could have a default value to the reason. However, I think defaulting a reason would be a footgun. I'd prefer to specify that the ctor errors out if no reason is provided. Wdyt?

@jan-ivar
Copy link
Member

Isn't this what permissions.query is for?

@eladalon1983
Copy link
Member Author

eladalon1983 commented Dec 12, 2023

Isn't this what permissions.query is for?

A number of things were said during the interim meeting just now, but for me the most compelling is that Permissions.query() is asynchronous, which means that checking it immediately after getDisplayMedia() rejects, is not guaranteed to result in anything truly correlated to the preceding call to getDisplayMedia().

@jan-ivar
Copy link
Member

jan-ivar commented Jan 9, 2024

... when the user tries to avail oneself of the option, it turns out that the browser is lacking the necessary OS permissions.

... both Chrome and Firefox set the message to "Permission denied by system". But that's not specified.

FYI Firefox rejects with NotFoundError: The object can not be found here. in this situation.

We find support for this in: "If no sources of type T are available, reject p with a new DOMException object whose name attribute has the value NotFoundError."

We take it to mean what's "available" to the user agent. Firefox does the same for camera and microphone.

This is a bit of a fingerprinting vector, so we're open to discussing changing this.

@eladalon1983
Copy link
Member Author

This is a bit of a fingerprinting vector, so we're open to discussing changing this.

Shall we go with this proposal then? IIRC, @youennf was on board with that too.

@youennf
Copy link
Collaborator

youennf commented Jan 9, 2024

NotFoundError allows distinguishing from NotAllowedError and is already supported by the spec.
It seems it fits the bill without any additional spec work.

@eladalon1983
Copy link
Member Author

Both NotAllowedError and NotFoundError are presently used by the spec, and can therefore not be used to distinguish this case.

@eladalon1983
Copy link
Member Author

Both NotAllowedError and NotFoundError are presently used by the spec, and can therefore not be used to distinguish this case.

When would Chrome return NotAllowedError for getDisplayMedia today?

When the dialog is shown and user presses cancel or escape.

@jan-ivar
Copy link
Member

jan-ivar commented Jan 9, 2024

Sorry I mistyped. I meant to ask, when would Chrome return NotFoundError from getDisplayMedia today?

@jan-ivar
Copy link
Member

jan-ivar commented Jan 9, 2024

Permissions.query() is asynchronous, which means that checking it immediately after getDisplayMedia() rejects, is not guaranteed to result in anything truly correlated to the preceding call to getDisplayMedia().

If query() still yields "prompt" then the app knows it has permission and the user canceled the prompt
If query() yields "blocked" then it doesn't know this, but knows it can't call again without user intervention

I'd like to understand when this is insufficient. I'm ignoring a user somehow managing to reconfigure their permissions (in the OS or browser) at the exact same time they also somehow manage to dismiss the prompt.

@eladalon1983
Copy link
Member Author

eladalon1983 commented Jan 9, 2024

when would Chrome return NotFoundError from getDisplayMedia today?

I'd have to check. Assume, for the sake of argument, that Chrome does not currently use NotFoundError for anything gDM-related. Is this real ideal? Would Web developers not be confused to find NotAllowedError for user-cancelled and NotFoundError for OS-blocked? Should we not provide something clearer? (To clarify - I am only proposing to change the spec around the latter.)

@youennf
Copy link
Collaborator

youennf commented Jan 9, 2024

The spec is already talking about OS/program/webpage lock prevents access.
We could add a note in the section that returns NotFoundError to state that OS preventing all access to display surfaces is a reason for not having any source and leads to NotFoundError.

Firefox does the same for camera and microphone.

Somehow related to this thread but we can note that, for getUserMedia, NotFoundError is ambiguous between no device and OS blocking access.
Plus it would be interesting to understand what enumerateDevices() should return in case OS prevents access to camera/microphone.

@eladalon1983
Copy link
Member Author

I wonder if a subclass, GetDisplayMediaNotAllowedError : DOMException, that has its name set to NotFoundError, might be a good way to give developers something clearer while keeping the existing Firefox behavior intact?

@jan-ivar
Copy link
Member

... it would be interesting to understand what enumerateDevices() should return in case OS prevents access to camera/microphone.

This is known. Both Chrome and Firefox report 1 camera and 1 microphone without macOS permission.

Somehow related to this thread but we can note that, for getUserMedia, NotFoundError is ambiguous between no device and OS blocking access.

Apps can disambiguate NotFoundError using enumerateDevices(). But yes gUM here is a side-track.

@jan-ivar
Copy link
Member

We could add a note in the section that returns NotFoundError to state that OS preventing all access to display surfaces is a reason for not having any source and leads to NotFoundError.

I like this idea.

I wonder if a subclass, GetDisplayMediaNotAllowedError : DOMException, that has its name set to NotFoundError, might be a good way to give developers something clearer while keeping the existing Firefox behavior intact?

What would this solve? Inheriting off DOMException is normally only done to add members to the interface.

@jan-ivar jan-ivar added this to the Candidate Recommendation milestone Feb 14, 2024
@dontcallmedom-bot
Copy link

This issue had an associated resolution in WebRTC February 2024 meeting – 20 February 2024 (Distinguish cancellations from absent OS permissions #281):

RESOLUTION: Move forward with a PR to clarify that NotFoundError would apply for OS permissions in screen-capture

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants