This repository was archived by the owner on Feb 6, 2026. It is now read-only.
Open
Conversation
The previous code is broken: the result of indexOf is an index, and -1 if the searched string is not found. So the value will be non-zero (and therefore thuthy) in all realistic setups, making the plugin enumeration an unused code branch. We COULD fix the intent of the original code, by comparing the indexOf result against -1. But doing so would break systems where WebMIDI is provided by some other plugin or polyfill, where plugins cannot be enumerated, where native functions stringify in some other way, where something monkey-patched the method in question, or something like that. So instead this commit here relies on the ducktyping philosophy: if there is a navigator.requestMIDIAccess method, we assume it does the right thing and avoid further checks. This is what the current code is essentially doing right now anyway, even though not intentionally.
gagern
added a commit
to gagern/CindyJS
that referenced
this pull request
Aug 1, 2016
The MIDI.js verison here is one which integrates three of my PRs: mudcube/MIDI.js#187 (percussion) mudcube/MIDI.js#189 (MIDI.now) mudcube/MIDI.js#190 (WebMIDI detection) The playtone function automatically triggers loading of the plugin. The corresponding soundfont is loaded only when needed, which means there will be some time between when the function calls start and when sound becomes actually audible. Samples are hosted on http://cindyjs.org/soundfont/ which might break widgets delivered through HTTPS. We need a certificate for cindyjs.org to solve this problem, or allow configuring the soundfont through some API, e.g. some CindyScript function. The example is essentially taken from a Cinderella export, except for the deletion of a duplicate play button.
gagern
added a commit
to gagern/CindyJS
that referenced
this pull request
Aug 1, 2016
The MIDI.js verison here is one which integrates three of my PRs: mudcube/MIDI.js#187 (percussion) mudcube/MIDI.js#189 (MIDI.now) mudcube/MIDI.js#190 (WebMIDI detection) The playtone function automatically triggers loading of the plugin. The corresponding soundfont is loaded only when needed, which means there will be some time between when the function calls start and when sound becomes actually audible. Samples are hosted on http://cindyjs.org/soundfont/ which might break widgets delivered through HTTPS. We need a certificate for cindyjs.org to solve this problem, or allow configuring the soundfont through some API, e.g. some CindyScript function. The example is essentially taken from a Cinderella export, except for the deletion of a duplicate play button.
gagern
added a commit
to gagern/CindyJS
that referenced
this pull request
Aug 1, 2016
The MIDI.js verison here is one which integrates three of my PRs: mudcube/MIDI.js#187 (percussion) mudcube/MIDI.js#189 (MIDI.now) mudcube/MIDI.js#190 (WebMIDI detection) The playtone function automatically triggers loading of the plugin. The corresponding soundfont is loaded only when needed, which means there will be some time between when the function calls start and when sound becomes actually audible. Samples are hosted on http://cindyjs.org/soundfont/ which might break widgets delivered through HTTPS. We need a certificate for cindyjs.org to solve this problem, or allow configuring the soundfont through some API, e.g. some CindyScript function. The example is essentially taken from a Cinderella export, except for the deletion of a duplicate play button.
montaga
pushed a commit
to montaga/CindyJS
that referenced
this pull request
May 25, 2018
The MIDI.js verison here is one which integrates three of my PRs: mudcube/MIDI.js#187 (percussion) mudcube/MIDI.js#189 (MIDI.now) mudcube/MIDI.js#190 (WebMIDI detection) The playtone function automatically triggers loading of the plugin. The corresponding soundfont is loaded only when needed, which means there will be some time between when the function calls start and when sound becomes actually audible. Samples are hosted on http://cindyjs.org/soundfont/ which might break widgets delivered through HTTPS. We need a certificate for cindyjs.org to solve this problem, or allow configuring the soundfont through some API, e.g. some CindyScript function. The example is essentially taken from a Cinderella export, except for the deletion of a duplicate play button.
montaga
pushed a commit
to montaga/CindyJS
that referenced
this pull request
May 25, 2018
The MIDI.js verison here is one which integrates three of my PRs: mudcube/MIDI.js#187 (percussion) mudcube/MIDI.js#189 (MIDI.now) mudcube/MIDI.js#190 (WebMIDI detection) The playtone function automatically triggers loading of the plugin. The corresponding soundfont is loaded only when needed, which means there will be some time between when the function calls start and when sound becomes actually audible. Samples are hosted on http://cindyjs.org/soundfont/ which might break widgets delivered through HTTPS. We need a certificate for cindyjs.org to solve this problem, or allow configuring the soundfont through some API, e.g. some CindyScript function. The example is essentially taken from a Cinderella export, except for the deletion of a duplicate play button.
montaga
pushed a commit
to montaga/CindyJS
that referenced
this pull request
May 30, 2018
The MIDI.js verison here is one which integrates three of my PRs: mudcube/MIDI.js#187 (percussion) mudcube/MIDI.js#189 (MIDI.now) mudcube/MIDI.js#190 (WebMIDI detection) The playtone function automatically triggers loading of the plugin. The corresponding soundfont is loaded only when needed, which means there will be some time between when the function calls start and when sound becomes actually audible. Samples are hosted on http://cindyjs.org/soundfont/ which might break widgets delivered through HTTPS. We need a certificate for cindyjs.org to solve this problem, or allow configuring the soundfont through some API, e.g. some CindyScript function. The example is essentially taken from a Cinderella export, except for the deletion of a duplicate play button.
montaga
pushed a commit
to montaga/CindyJS
that referenced
this pull request
May 30, 2018
The MIDI.js verison here is one which integrates three of my PRs: mudcube/MIDI.js#187 (percussion) mudcube/MIDI.js#189 (MIDI.now) mudcube/MIDI.js#190 (WebMIDI detection) The playtone function automatically triggers loading of the plugin. The corresponding soundfont is loaded only when needed, which means there will be some time between when the function calls start and when sound becomes actually audible. Samples are hosted on http://cindyjs.org/soundfont/ which might break widgets delivered through HTTPS. We need a certificate for cindyjs.org to solve this problem, or allow configuring the soundfont through some API, e.g. some CindyScript function. The example is essentially taken from a Cinderella export, except for the deletion of a duplicate play button.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The previous code is broken: the result of
indexOfis an index, and-1if the searched string is not found. So the value will be non-zero (and therefore thuthy) in all realistic setups, making the plugin enumeration an unused code branch.We could fix the intent of the original code, by comparing the
indexOfresult against-1. But doing so would break systems where WebMIDI is provided by some other plugin or polyfill, where plugins cannot be enumerated, where native functions stringify in some other way, where something monkey-patched the method in question, or something like that.So instead this commit here relies on the ducktyping philosophy: if there is a
navigator.requestMIDIAccessmethod, we assume it does the right thing and avoid further checks. This is what the current code is essentially doing right now anyway, even though not intentionally.