Re: [geolocation-api] errorCallback parameter to getPosition() and watchPosition() is nullable in all major browser engines (#61)

The questions of nullability and optionality are completely independent.  That is, in general you can have a thing that is neither one, a thing that is nullable but not optional, a thing that is optional but not nullable, or a thing that is both nullable and optional.

Dictionaries are a little weird in that dictionary arguments are _required_ to be optional and _forbidden_ from being nullable (because `null` is a value that converts to an empty dictionary anyway).  But for most other types, the setup above applies, and you just pick the option that represents the behavior you want.

So the questions to answer to decide on nullable/optional bits are:

1. If null is passed, should this throw an exception?  If not, the type needs to be nullable, generally.
2. Is omitting the argument entirely OK?  If it is, the type should be optional.

In this case, if both `watchPosition(() => {})` and `watchPosition(() => {}, null)` are meant to be acceptable, you want the argument both nullable and optional.  If the first should be OK but the second not, you want optional and not nullable.  And so forth.

Past that, if it's optional you may want to define a default value, or just define the behavior for the missing-argument case in prose; the former is preferred if the desired behavior for missing-argument case is the same as the behavior for some value of the argument.

Conceretely, the IDL in Gecko for `watchPosition` looks like this:
```
  long watchPosition(PositionCallback successCallback,
                     optional PositionErrorCallback? errorCallback = null,
                     optional PositionOptions options = {});
```
and the IDL in Blink looks like this:
```
  long watchPosition(
        PositionCallback successCallback,
        // TODO(crbug.com/841185): |errorCallback| is not nullable in the spec.
        optional PositionErrorCallback? errorCallback,
        optional PositionOptions options = {});
```
If you read https://bugs.chromium.org/p/chromium/issues/detail?id=841185 it says:
> Web IDL optional is meant to accept only ES undefined, and Blink follows this rule in general except for callback arguments.  Historically Blink has not been throwing a TypeError when ES null is passed to optional non-nullable callback arguments.

which probably explains how the spec ended up in the state it's in: the IDL was copied from Blink (or added by a Blink engineer), and more or less depended on the Blink IDL bug described above.  The bug got fixed in their bindings, and the IDL was edited to preserve behavior, but the spec never got updated, apparently.

Now Blink _still_ has a bug in their bindings, as far as I can tell: a nullable optional callback is defaulted to null even if the IDL doesn't have a default value defined.  I filed https://bugs.chromium.org/p/chromium/issues/detail?id=1151949 on that.

-- 
GitHub Notification of comment by bzbarsky
Please view or discuss this issue at https://github.com/w3c/geolocation-api/issues/61#issuecomment-732286695 using your GitHub account


-- 
Sent via github-notify-ml as configured in https://github.com/w3c/github-notify-ml-config

Received on Monday, 23 November 2020 16:52:28 UTC