User gesture check should fail with some other error type (not SecurityError)?

[mgiuca]

@foolip reported here:

...if you feel like changing it, I'd argue that user gesture checks are not quite security sensitive, but rather prevent user annoyance. FWIW, https://fullscreen.spec.whatwg.org/ rejects with TypeError.

@sammc I think we borrowed the SecurityError from some other API, so there may be two different precedents. Do you remember which?

[mgiuca]

I think it might've been Web Bluetooth:

If the algorithm is not triggered by user activation, throw a SecurityError and abort these steps.

[sammc]

WebUSB too.

[mgiuca]

We found some more precedents:

• HTMLMediaElement.play -- NotAllowedError (not explicitly attached to a user gesture requirement but it does give that as an example at the user agent's discretion, and Chrome's test at least expects NotAllowedError in the absence of a user gesture).
• PresentationRequest.start -- InvalidAccessError.

In summary:

• Fullscreen: TypeError
• Web Bluetooth: SecurityError
• Web USB: SecurityError
• HTMLMediaElement: NotAllowedError
• PresentationRequest: InvalidAccessError

I think TypeError is an outlier and an inappropriate error type (if you look at the fullscreen spec, it seems to just be lazy in that there is a catch-all for all errors that throws TypeError).

SecurityError is "The operation is insecure" which does seem not quite right since it's just annoyance, but it seems a reasonable approximation.

InvalidAccessError is deprecated according to WebIDL.

NotAllowedError seems like the most appropriate: "The request is not allowed by the user agent or the platform in the current context, possibly because the user denied permission." I'd be happy to change to NotAllowedError for that, citing HTMLMediaElement as a precedent.

@jyasskin @reillyeon Is this something you've considered for Web Bluetooth, Web USB respectively?

[foolip]

if you look at the fullscreen spec, it seems to just be lazy in that there is a catch-all for all errors that throws TypeError

This is true, but not accidental, all specs written by @annevk use TypeError for everything.

[annevk]

I don't think calling Fullscreen "lazy" is reasonable justification for having different exceptions.

[jyasskin]

I think we should try to throw a consistent error from user gesture checks, and we should probably specify which one in the Permissions spec. Starting from scratch, I wouldn't think TypeError makes sense—it may be the generic ECMAScript error, but WebIDL-using specs have all the DOMException types to give developers more useful distinctions.

One could argue that consistency with the most widely used spec, Fullscreen, means that we should accept the error type used there, even if it's widely agreed to be a worse option. I'm not currently convinced by that argument.

I agree that NotAllowedError ("The request is not allowed by the user agent or the platform in the current context, possibly because the user denied permission.") is a better option than SecurityError ("The operation is insecure."). I think I used SecurityError in Web Bluetooth, which Web USB copied, because NotAllowedError didn't exist yet. I suspect it's not too late to change that in Web Bluetooth and Web USB: @scheib, what do you think? Importantly, this is an error that always indicates an application bug, since control flow should start from a gesture. Since developers should never need to catch the exception and recover, changing its name shouldn't break any applications.

[scheib]

Changing Web Bluetooth and WebUSB ( @reillyeon ) is very practical since they have launched so recently. Pointer Lock should use whatever Fullscreen does. I don't think it matters much which error type is used, (apps consistently won't be working at all no matter which type is used).

[annevk]

I'm not convinced that just because IDL gives you lots of options, that you necessarily need to use them. The pattern established by ECMAScript is not really distinguish much between different types of exceptions. And you rarely if ever see try/catch branching on exceptions.

[jyasskin]

@annevk We need to get the guidance to use TypeError for everything into WebIDL if we want the platform to consistently do it. Otherwise, it winds up being just your specs that are inconsistent with everything else.

[mgiuca]

Given that Fullscreen and HTMLMediaElement are the only two that have been properly standardized, we should change the non-standard ones to match one of those if they can be.

I don't think calling Fullscreen "lazy" is reasonable justification for having different exceptions.

Sure, sorry. The reason to not use TypeError is that in the context of Web IDL (i.e., web standards, not just generic JavaScript), we have a wide variety of error types to choose and we should choose the most appropriate one. TypeError is defined by ECMAScript as a generic catch-all and (although not explicitly defined as such) seems to be used for when either the type or value of an argument is incorrect, not for when a method was called from an incorrect context.

WebIDL doesn't really give advice on TypeError but there's a hint in its definition of InvalidAccessError:

Deprecated. Use TypeError for invalid arguments, "NotSupportedError" DOMException for unsupported operations, and "NotAllowedError" DOMException for denied requests instead.

That suggests that NotAllowedError is correct here.

If there was already an established pattern of using TypeError for this, I'd go with it, but there seems to be no established pattern, and no good reason to pick TypeError over NotAllowedError.

Pointer Lock should use whatever Fullscreen does.

Unfortunately, Pointer Lock has a very weird mechanism (possibly because it pre-dates promises) --- it fires a "pointerlockerror" event instead of having any exception type. So that is kind of irrelevant here.

[annevk]

@domenic thoughts on changing guidance in IDL? I'm guessing you don't care much either way. I also don't care too much, but I rather avoid using DOMException when I can to make it easier to port APIs to Node.js (as is happening with URL and TextDecoder and such).

[domenic]

TypeError is defined by ECMAScript as a generic catch-all and (although not explicitly defined as such)

It is explicitly defined as such: "TypeError is used to indicate an unsuccessful operation when none of the other NativeError objects are an appropriate indication of the failure cause."

I generally agree that we haven't seen web developers distinguish on error types much, and using more specific ones isn't generally helpful. But, I don't care that much. Either TypeError or "NotAllowedError" DOMException seem OK, in specs that have no chance of being ported outside a browser environment.

[mgiuca]

It is explicitly defined as such: "TypeError is used to indicate an unsuccessful operation when none of the other NativeError objects are an appropriate indication of the failure cause."

Yes but that is for native errors (i.e., errors that are generated by the ECMAScript runtime system). It is clearly not suggesting that all errors in a JavaScript program or library that are not {EvalError, RangeError, ReferenceError, SyntaxError, URIError} be TypeError (and DOM is really just a JavaScript library).

I also don't care too much, but I rather avoid using DOMException when I can to make it easier to port APIs to Node.js (as is happening with URL and TextDecoder and such).

That's a good point but I agree with Domenic; there is a distinction between APIs that are part of the web platform and don't make sense outside of that environment, versus more generic "utility" APIs such as URL and TextDecoder that can slot into any JavaScript environment. I agree that the latter should avoid using DOMExceptions.

Since we're talking about a rule for "what exception type do you throw when an API is called without a user gesture", that will only ever apply to the former category of API, so I think it's safe to use a DOMException type here.

[foolip]

FWIW, I'd support that change in the Fullscreen spec. Even if web developers don't distinguish between exceptions, it can still be helpful in tests, to make sure you've hit the right error condition.

[mgiuca]

I've changed navigator.share's type from SecurityError to NotAllowedError.

If some consensus is reached to change other APIs to something other than NotAllowedError, we can change navigator.share (which will be a small breaking change, but there would be breaking changes happening to other APIs at the same time).

[foolip]

Filed a bug for fullscreen. Which specs would remain, then?

[mgiuca]

I've done a bit of a more exhaustive survey (just grepping through the Chromium source for ProcessingUserGesture).

• Bluetooth: SecurityError
• Fullscreen: TypeError
• HTMLMediaElement: NotAllowedError
• PresentationRequest: InvalidAccessError
• RemotePlayback: InvalidAccessError
• USB: SecurityError
• VRDisplay: InvalidStateError (Chromium-specific; the spec says it must reject but doesn't say which error to use)

This isn't necessarily exhaustive but it's the best I can find from looking through the Chromium source.

[foolip]

Thanks, that's quite the list, and quite a zoo of exceptions for the same thing. Bringing consistency to this seems possible, but the question is how useful it'd be. @mgiuca, do you feel inclined to go change all these specs and write tests, or leave it be?

[mgiuca]

@foolip My feeling is that it's not worth changing, especially the well-established ones like Fullscreen. The ones that haven't yet been standardized, maybe.

I don't want to commit myself to changing all the specs! But I'm happy to file bugs against the relevant standards. Let's just leave the fullscreen one and have a discussion either there, or here, before we go spamming all the bug trackers.

[foolip]

So, for Fullscreen I'm now inclined to leave it alone, unless someone else volunteers to make both the spec change, tests and the change to Chromium. Will close that issue on the assumption that no one will, but some one can reopen.