can't use MUST on scaleResolutionDownBy input

[jan-ivar]

http://w3c.github.io/webrtc-pc/#widl-RTCRtpEncodingParameters-scaleResolutionDownBy says: "The value must be greater than 0." which isn't actionable.

The spec should instead specify what MUST happen when the input value is 0 or less. Throw a TypeError? ignore?

[alvestrand]

Should it be restricted to > 1.0? ScaleResolutionDownBy=0.0001 seems a bit dangerous..

[alvestrand]

@burnburn what's the error most fitting for "this parameter has a stupid value"?

[jan-ivar]

@alvestrand I agree.

@burnburn There's a precedent for TypeError here https://heycam.github.io/webidl/#EnforceRange.

[jan-ivar]

Or maybe that's a stretch, since [EnforceRange] are about what fits in a type, not further limiting values to what makes sense in a context?

[burnburn]

Or we could use SyntaxError (see http://heycam.github.io/webidl/#idl-DOMException-error-names). I think either works.

[jan-ivar]

I think SyntaxError is traditionally for (string) parsers, and the simple exception by the same name "is for use only by the ECMAScript parser". Also, it's message reads "The string did not match the expected pattern." which seems wrong for a number range check. Odd that there's no DOMException equivalent to RangeError.

[jan-ivar]

InvalidModificationError?

[jan-ivar]

On a side-note I'm a bit amused that there's apparently a ConstraintError DOMException name now...

[jan-ivar]

Our DOM folks suggested throwing a simple RangeError. Apparently that's still done in places:

https://encoding.spec.whatwg.org/#dom-textencoder
https://fetch.spec.whatwg.org/#dom-response

[aboba]

InvalidParameter exception?

[jan-ivar]

Where is that defined?

[burnburn]

There is no InvalidParameter here (http://heycam.github.io/webidl/#idl-DOMException-error-names). I wish there were!

Anyway, I'm okay with RangeError unless/until a better suggestion occurs. I was aware of the SyntaxError being primarily for strings, but we are clearly having to stretch definitions since we are limited in the list of errors.

Jan-Ivar, I won't object if you submit a PR with RangeError. If you'd rather I do it, let me know.

[jan-ivar]

Sure, would this go in the processing model of setParameters or as a note under scaleResolutionDownBy?

Funny, I see setParameters already rejects with RangeError for modifying read-only parameters:

"If any parameter in the parameters argument, marked as a Read-only parameter, has a value that is different from the corresponding parameter value returned from getParameters(), abort these steps and return a promise rejected with RangeError. "

Is that an issue (reusing the same error)?

[jan-ivar]

Maybe the read-only error should be InvalidModificationError ("The object can not be modified in this way."), so there is some differentiation, and then we can use RangeError for scaleResolutionDownBy? That seems maybe more logical to me. @burnburn, @aboba - thoughts?

[burnburn]

Agreed. I was just about to type exactly the same thing. InvalidModificationError for the read-only, and RangeError for the bad value.

[aboba]

@burnburn - Agree that we should use a distinct error for a bad value as opposed to an invalid modification.
@jan-ivar - Agree that RangeError is probably not what we want to return in response to modifying read-only parameters, because the issue is not that the parameter is out-of-range. For example, we are using InvalidModificationError to indicate an SDP modification forbidden by JSEP Section 6, and the reason we cannot modify a read-only parameter is that these parameters require an SDP O/A so the situation is somewhat similar.

[aboba]

@alvestrand - I agree that scaleResolutionDownBy should be required to be greater than or equal to 1, not just greater than 0.

[aboba]

I think we can close this now.