This is an archived snapshot of W3C's public bugzilla bug tracker, decommissioned in April 2019. Please see the home page for more details.

Bug 20376 - Change fingerprint to id
Summary: Change fingerprint to id
Status: CLOSED FIXED
Alias: None
Product: AudioWG
Classification: Unclassified
Component: MIDI API (show other bugs)
Version: unspecified
Hardware: PC Linux
: P2 normal
Target Milestone: TBD
Assignee: Jussi Kalliokoski
QA Contact: public-audio
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-12-13 15:07 UTC by Jussi Kalliokoski
Modified: 2013-01-02 11:34 UTC (History)
3 users (show)

See Also:


Attachments

Description Jussi Kalliokoski 2012-12-13 15:07:54 UTC
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.
Comment 1 Jussi Kalliokoski 2012-12-13 15:35:39 UTC
(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
Comment 2 Jussi Kalliokoski 2012-12-13 15:46:27 UTC
-- 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'.
Comment 3 Jussi Kalliokoski 2012-12-13 16:20:32 UTC
(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.
Comment 4 Chris Wilson 2012-12-13 19:18:36 UTC
Restructuring this bug to only represent fingerprint naming.
Comment 5 Chris Wilson 2012-12-13 19:34:28 UTC
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.
Comment 6 Olivier Thereaux 2013-01-02 11:34:28 UTC
(In reply to comment #5)
> 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.

Seeing no objection on either this or https://www.w3.org/Bugs/Public/show_bug.cgi?id=19803, closing.