Change fingerprint to id

[jussi-kalliokoski]

Originally reported on W3C Bugzilla ISSUE-20376 Thu, 13 Dec 2012 15:07:54 GMT Reported by Jussi Kalliokoski Assigned to Jussi Kalliokoski

On Thursday, December 13, 2012 at 1:53 PM, Marcos Caceres wrote:

Obtains an interface to enumerate and request access to MIDI devices on the user's system.

This call may prompt the user for access to MIDI devices. The above needs to be a SHOULD. If the user accepts

accepts should be "If the user gives express permission"

or the call is otherwise approved, successCallback is invoked, with a MIDIAccess object as its argument.

If the user declines or the call is denied, the errorCallback (if any) is invoked. All the above should really be in the algorithm or all this should be labelled as non-normative (i.e., this is a note of how it works conceptually, but can't be implemented).

When the getMIDIAccess() method is called, the user agent must run the following steps:

Let successCallback be the callback indicated by the method's first argument.

Let errorCallback be the callback indicated by the method's second argument, if any, or null otherwise. I think it' redundant for NavigatorMIDIAccessErrorCallback to be nullable when it's already optional. If successCallback is null, abort these steps.

This is wrong. Error handling is handled by WebIDL. Having this here conflicts with WebIDL's behavior.

Return, and run the following steps asynchronously.

Optionally, e.g. based on a previously-established user preference, for security reasons, or due to platform limitations, jump to the step labeled failure below.

Optionally, e.g. based on a previously-established user preference, jump to the step labeled success below.

Prompt the user in a user-agent-specific manner for permission to provide the entry script's origin with a MIDIAccess object representing control over user's MIDI devices.

If permission is denied, jump to the step labeled failure below. If the user never responds, this algorithm stalls on this step.

success: Let access be the MIDIAccess object for which access has been granted.

Queue a task to invoke successCallback with access as its argument, and return.

Failure: If errorCallback is null, abort these steps. Better would be to say, "if errorCallback is a callable". WebIDL would have caught other types already.

Let error be a new NavigatorMIDIAccessError object whose code attribute has the numeric value 1 (PERMISSION_DENIED ). Change this to DOMError (this is either a "securityError" or "NotSupportedError" depending on situation). Please don't introduce new Error types into the platform.

I have another email about this… but basically, you don't need NavigatorMIDIAccessError. Please delete it.

Queue a task to invoke errorCallback with error as its argument.

The task source for these tasks is the user interaction task source.

Above seems ok.

[jussi-kalliokoski]

Original comment by Jussi Kalliokoski on W3C Bugzilla. Thu, 13 Dec 2012 15:35:39 GMT

(In reply to comment #0)

The above needs to be a SHOULD.

Fixed: https://dvcs.w3.org/hg/audio/rev/bf4f6fd3ecef

accepts should be "If the user gives express permission"

Fixed: https://dvcs.w3.org/hg/audio/rev/bf4f6fd3ecef

All the above should really be in the algorithm or all this should be labelled as non-normative (i.e., this is a note of how it works conceptually, but can't be implemented).

Not fixed yet, not sure which is better.

I think it' redundant for NavigatorMIDIAccessErrorCallback to be nullable when it's already optional.

Fixed: https://dvcs.w3.org/hg/audio/rev/bf4f6fd3ecef

This is wrong. Error handling is handled by WebIDL. Having this here conflicts with WebIDL's behavior.

Fixed: https://dvcs.w3.org/hg/audio/rev/bf4f6fd3ecef

Better would be to say, "if errorCallback is a callable". WebIDL would have caught other types already.

Fixed: https://dvcs.w3.org/hg/audio/rev/bf4f6fd3ecef

Change this to DOMError (this is either a "securityError" or "NotSupportedError" depending on situation). Please don't introduce new Error types into the platform.

I have another email about this… but basically, you don't need NavigatorMIDIAccessError. Please delete it.

Done: https://dvcs.w3.org/hg/audio/rev/bf4f6fd3ecef

[jussi-kalliokoski]

Original comment by Jussi Kalliokoski on W3C Bugzilla. Thu, 13 Dec 2012 15:46:27 GMT

-- More from Marcos --

Some more rapid fire feedback :)

Bikeshed: getMIDIAccess is a misnomer. The developer is not guaranteed access to the MIDI interface, so it's a "request". Hence, this method should be renamed to "requestMIDIAccess()" or just "requestMIDI()".

The following is also incorrect: [TreatNonCallableAsNull] attribute callback? onmessage;

Please change it to: attribute EventHandler onmessage;

It would be better if you could fold everything into MIDIPort and get rid of MIDIOutput and MIDIInput? you already have the port type, and you can just say that sending() does nothing when a port is not outputting.

If you don't agree, then I think MIDIInput and MIDIOutput need to inherit from MIDIPort (not implement the interface). Implementing the interface makes a huge mess when actually implementing, as the stuff from MIDIPort has to be copied over from MIDIPort.

So, worst case, please change the spec to match the following pattern:

interface MIDIOutput : MIDIPort { } interface MIDIInput : MIDIPort { } MIDIPort : EventTarget{ }

However, I strongly urge you to do away with MIDIInput and MIDIOutput. They are redundant, IMHO.

I have a strong concerns about exposing the manufacturer and fingerprint attributes. Adding the manufacturer encourages device specific programming, which is bad (it also serves as a strong vector for fingerprinting).

I understand the use case for the fingerprint attribute, but I think that use case should be handled by the UA and not exposed to the developer. I'm not sure what the right solution is here either, but what is currently there does not feel right.

Bikeshed: fingerprint sounds creepy. Please change it to 'id'.

[jussi-kalliokoski]

Original comment by Jussi Kalliokoski on W3C Bugzilla. Thu, 13 Dec 2012 16:20:32 GMT

(In reply to comment #2)

Bikeshed: getMIDIAccess is a misnomer. The developer is not guaranteed access to the MIDI interface, so it's a "request". Hence, this method should be renamed to "requestMIDIAccess()" or just "requestMIDI()".

Good point, requestMIDIAccess is more accurate. I changed it accordingly. I think requestMIDI sounds a bit weird, does it request a MIDI spec? Protocol? :D Anyway: https://dvcs.w3.org/hg/audio/rev/ddd6455eebf6

I have a strong concerns about exposing the manufacturer and fingerprint attributes. Adding the manufacturer encourages device specific programming, which is bad (it also serves as a strong vector for fingerprinting).

I understand the use case for the fingerprint attribute, but I think that use case should be handled by the UA and not exposed to the developer. I'm not sure what the right solution is here either, but what is currently there does not feel right.

More discussion of the use cases of the "fingerprint" in this bug: https://www.w3.org/Bugs/Public/show_bug.cgi?id=19803

Anyway, I understand where you're coming from with this, but at least exposing the name and fingerprint are crucial features to have around in order to support good user experience. The name so that the user can choose what device to use for what purpose (a lot of the end users of this API will have multiple MIDI devices they will want to use for different purposes), and the fingerprint so that the application can maintain the tasks the user assigns to devices across sessions. I don't really see a good way for this to be completely handled by the UA without making a really complex API and possibly bad user experience.

Admittedly, the rest of the attributes (version and manufacturer) serve less purpose, provided that the fingerprint works. But they're not entirely pointless, after all, MIDI already has sysex and different versions of different devices from different manufacturers might have different ways of, for example, uploading new synth patches to the device, so device-specific programming is a bit difficult to avoid. Especially since I expect that one prime audience for this API will be manufacturers who want to give users an easy way to manage the device they bought ("Open your browser and head to this address to modify your device's settings, no install required!").

Bikeshed: fingerprint sounds creepy. Please change it to 'id'.

Heh, you're probably right about that.

[jussi-kalliokoski]

Original comment by Chris Wilson on W3C Bugzilla. Thu, 13 Dec 2012 19:18:36 GMT

Restructuring this bug to only represent fingerprint naming.

[jussi-kalliokoski]

Original comment by Chris Wilson on W3C Bugzilla. Thu, 13 Dec 2012 19:34:28 GMT

Renamed to id. https://dvcs.w3.org/hg/audio/rev/e28ee1b5d990

Marcos, you may wish to comment on/reopen the fingerprint issue - https://www.w3.org/Bugs/Public/show_bug.cgi?id=19803.