fix(cddl): fix CDDL syntax for emulation.SetGeolocationOverrideParameters

[christian-bromann]

I find the current definition of emulation.SetGeolocationOverrideParameters somewhat unusual and inconsistent with the style used throughout the rest of the specification.

emulation.SetGeolocationOverrideParameters = {
   (
      (coordinates: emulation.GeolocationCoordinates / null) //
      (error: emulation.GeolocationPositionError)
   ),
   ? contexts: [+browsingContext.BrowsingContext],
   ? userContexts: [+browser.UserContext],
}

Aside from concerns about whether this is valid CDDL syntax—and the fact that some parsers may or may not support it—I recommend refactoring this definition to better align with the established structure used elsewhere in the spec. Specifically, we typically define separate types for success and error cases, then unify them with a top-level choice. For example:

script.EvaluateResult = (
  script.EvaluateResultSuccess /
  script.EvaluateResultException
)

script.EvaluateResultSuccess = {
  type: "success",
  result: script.RemoteValue,
  realm: script.Realm
}

script.EvaluateResultException = {
  type: "exception",
  exceptionDetails: script.ExceptionDetails,
  realm: script.Realm
}

Applying a similar pattern to the emulation namespace would improve readability, tooling compatibility, and consistency across the specification.

---

Preview | Diff

[whimboo]

Thanks @christian-bromann. Personally I definitely like that better. I've just a question that you can find inline.

CC @sadym-chromium and @jgraham

[whimboo]

AFAICS this can stay as a map, or do I miss something?

[OrKoN]

Right, I think this should stay a map

[christian-bromann]

While it is possible to insert a group into a map, e.g.:

foobar = {
  someGroup,
  key: value
}

It is not possible to include group choices, because the map expects a single group, not a group choice expression*.

RFC 8610 (Section 3.5) says:

When a group is used as a member of a map, the entries in the group are included at that location in the map.

But that refers to a group, not a group choice. Group choices (//) are only valid inside a group.

[OrKoN]

(emulation.SetGeolocationPosition // emulation.GeolocationPositionError) defines a new group (due to ()) so it is valid to put it into a map. We have other places where this works. The current PR version does provide any type for the params key.

[christian-bromann]

@OrKoN it is not allowed to have a group choice (//) between two maps ({ ... }). This aligns with other places in the spec, e.g.

• (browser.ClientWindowNamedState // browser.ClientWindowRectState) (ref) where both browser.ClientWindowNamedState and browser.ClientWindowRectState are groups (( ... ))
• (network.ContinueWithAuthCredentials // network.ContinueWithAuthNoCredentials) (ref) where also both network.ContinueWithAuthCredentials and network.ContinueWithAuthNoCredentials are groups

The CDDL specification indirectly defines this in RFC 8610 §3.9 — Group-to-Group Composition:

The operator // is used to express a choice between groups. The right-hand side of the operator must be a group.

This means:

• A // B is only valid if both A and B are groups.
• A map ({ ... }) is a type, not a group, and therefore cannot be used directly in //.

[OrKoN]

Sorry, I am not following: both SetGeolocationPosition and GeolocationPositionError are groups, not maps. SetGeolocationOverrideParameters has to remain a map:

emulation.SetGeolocationOverrideParameters = {
      ? contexts: [+browsingContext.BrowsingContext],
      ? userContexts: [+browser.UserContext],
     (emulation.SetGeolocationPosition // emulation.GeolocationPositionError)
}