Determine how set() will work for "flag" mode + scope

[w3c]

Determine how set() will work for "flag" mode + scope

[mgiuca]

Wait, it turns out WebIDL has overloading. Good, then we should simply be able to define it like this:

// Set badge to "flag".
static void set();
// Set badge to a number, optionally giving an options dictionary.
static void set([EnforceRange] unsigned long long contents,
                optional BadgeOptions options);
// Set badge to "flag", with an options dictionary.
static void set(BadgeOptions options);

But we have to read this very carefully to see how the type matching would work. Thanks @g-ortuno .

[fallaciousreasoning]

Note: When we've made the changes in #55 this will no longer be relevant.

[fallaciousreasoning]

FWIW

set(optional unsigned long long contents, optional BageOptions options);

seems to work in Chromium

https://cs.chromium.org/chromium/src/third_party/blink/renderer/modules/badging/experimental_badge.idl?q=f:badge.idl&sq=package:chromium&g=0&l=15.

As to style, I think you're right, we should talk to the TAG folks.

[g-ortuno]

In bluetooth we do:

typedef (DOMString or unsigned long) BluetoothServiceUUID;
getPrimaryServices(optional BluetoothServiceUUID);

Couldn't you do something similar?

typedef (unsigned long long or BadgeOptions) SetParam;
set(optional SetParam);

[g-ortuno]

There might be a way to avoid the typedef depending on how we parse the IDL.

[mgiuca]

FWIW

set(optional unsigned long long contents, optional BageOptions options);

seems to work in Chromium

It compiles but it doesn't work as intended.

https://crbug.com/1000968

Uncaught TypeError: Failed to execute 'set' on 'ExperimentalBadge': Value is not of type 'unsigned long long'.

Both parameters are optional (as in, you can pass 0 arguments), but if you do pass one, it always goes to the contents parameter. I assume this is correct according to WebIDL (it would be a bit crazy to expect this to automatically decide which parameter to fill based on the type).

[mgiuca]

@g-ortuno Yes, you can certainly have an "or" as a parameter (and the typedef is not required). But it's more complex than simply allowing one of two types in the first parameter slot, since you also need to prevent the dictionary from being passed as the first and second parameter. The closest approximation is:

static void set(optional (([EnforceRange] unsigned long long) or BadgeOptions) contentsOrOptions,
                optional BadgeOptions options);

Statically, this would allow:

1. set()
2. set(num)
3. set({scope: url})
4. set(num, {scope: url})
5. set({scope: url1}, {scope: url2})

And then we need code inside the method to throw an error if contentsOrOptions is an object and options is supplied. It can be done, but it's a bit messy.

[mgiuca]

Interesting, on the alternative approach where we ask for an explicit value in the first argument position, I think undefined should "just work":

Badge.set(undefined, {scope: url});

Based on a cursory reading of the (admittedly very long) JS-to-IDL overload resolution algorithm (which is the algorithm responsible for converting a JS argument list into WebIDL types), undefined should just select the default value when passed to an optional parameter, so the above should set a flag badge with a scope. But it doesn't work. Same as above:

Uncaught TypeError: Failed to execute 'set' on 'ExperimentalBadge': Value is not of type 'unsigned long long'.

This might be a bug in Chrome's IDL parser? (Edit: Filed bug on Chrome.)

Badge.set(undefined) sets a flag badge, but Badge.set(undefined, {scope: url}) gives a TypeError converting undefined to a long long, so ... maybe there's a bug in Chrome's overload resolution algorithm.

Nevertheless, I would prefer to allow you to pass the dictionary to the first parameter if that's possible.

[mgiuca]

Scope has been removed.