XHR Level 2 responseType of arrayBuffer and Blob #2433

Open
wants to merge 3 commits into
from

Conversation

Projects
None yet
5 participants

leegee commented Oct 8, 2012

This pull request in response to the below mails on the MT users' list.

The change is that a new option has been added to Requestjs, 'responseType.' Valid values are not checked, but passed directly to the xhr object just after it has been open()ed.

A callback could not be used to do this, as the xhr object is never exposed in an existing callback, and exposing it seemed contrary to the spirit of the code. Furthermore, this is base functionality of XHR, at level two, and is required for requests of HTML5 audio data via the Web Audio API (at least), so without this support, MooTools cannot be used with HTML5 Web Audio.

Please let me know what you think.

On 08/10/2012 05:09, Barry van Oudtshoorn wrote:

This sounds like it'd be something that could reasonably be exposed as an option on the Request class, so my recommendation would be to alter that class and send a pull request. No guarantee that it'll be accepted, of course, but this is certainly the way that I'd want to interact with this in MooTools. It's a pretty fundamental operation on XHR, to be honest, so I think it's worthwhile.

On 07/10/12 17:22, Lee Goddard wrote:

I'm making XHR requets with reponseType set to 'arraybuffer', so that I can receive binary data to pass to a Web Audio API audio context's decodeAudioData() method.

As far as I know, this cannot be done with MooTools as it stands, because although I could sub-class Request and adjust the onStateChange() and success(), access is required to the private XHR so that the responseType can be set after the requested is open()ed.

Over-riding the send() method seems like a bad idea, as I'd have to keep it updated with any change made to the core.

The only solution I can see is to expose the currently-private xhr object, but I guess it is private for a reason.

Does anyone have any suggestions, please?

TIA
Lee

Owner

arian commented Oct 24, 2012

👍 would be good to bring more XHR 2 features in.

Member

DimitarChristoff commented Oct 24, 2012

seems cool. i like it. 👍 still, you can easily create your own Request subclass with this func in. it seems like too much of an edgecase

leegee commented Jan 3, 2013

Dimitar,

Thing is that MT is falling behind in the world of XHR, and if I start creating my own custom Request sub-classes, I lose the benefit of using a globally-distributed library. I already have four projects with my own custom Request sub-class, and I'm not happy explaining to people why that is necessary.

This patch may not be good enough, but XHR Level 2 must be supported in MooTools if the library is survive in the world of HTML5, and unless Request is to be rewritten, I thought the approach I took did the least harm.

Owner

cpojer commented Jan 3, 2013

I like this a lot. @leegee would you mind amending your commit with proper MooTools coding standards? (There are too many spaces in the documentation code example and an else block belongs on the same line with the closing parenthesis of the previous block and the opening parenthesis of the else block).

Also, it seems to my like you could shorten the code block around defining the response. If you don't know what to do feel free to join #mootools on IRC and ping me :)

leegee commented Jan 3, 2013

Will do, thank you.

@arian arian commented on an outdated diff Jan 18, 2013

Source/Request/Request.js
@@ -74,11 +74,21 @@ var Request = this.Request = new Class({
if (progressSupport) xhr.onprogress = xhr.onloadstart = empty;
clearTimeout(this.timer);
- this.response = {text: this.xhr.responseText || '', xml: this.xhr.responseXML};
- if (this.options.isSuccess.call(this, this.status))
- this.success(this.response.text, this.response.xml);
- else
- this.failure();
+ if (this.options.responseType
+ && (this.options.responseType == 'arraybuffer' || this.options.responseType == 'blob') ){
@arian

arian Jan 18, 2013

Owner

Just put it on one line. Also there is an erroneous space at ) ){

@arian arian commented on an outdated diff Jan 18, 2013

Source/Request/Request.js
@@ -74,11 +74,21 @@ var Request = this.Request = new Class({
if (progressSupport) xhr.onprogress = xhr.onloadstart = empty;
clearTimeout(this.timer);
- this.response = {text: this.xhr.responseText || '', xml: this.xhr.responseXML};
- if (this.options.isSuccess.call(this, this.status))
- this.success(this.response.text, this.response.xml);
- else
- this.failure();
+ if (this.options.responseType
+ && (this.options.responseType == 'arraybuffer' || this.options.responseType == 'blob') ){
+ this.response = {};
+ this.response[this.options.responseType] = this.xhr.response || '';
+ if (this.options.isSuccess.call(this, this.status))
@arian

arian Jan 18, 2013

Owner

Add curly brackets around ifs.

@arian arian commented on an outdated diff Jan 18, 2013

Source/Request/Request.js
- this.response = {text: this.xhr.responseText || '', xml: this.xhr.responseXML};
- if (this.options.isSuccess.call(this, this.status))
- this.success(this.response.text, this.response.xml);
- else
- this.failure();
+ if (this.options.responseType
+ && (this.options.responseType == 'arraybuffer' || this.options.responseType == 'blob') ){
+ this.response = {};
+ this.response[this.options.responseType] = this.xhr.response || '';
+ if (this.options.isSuccess.call(this, this.status))
+ this.success(this.response[this.options.responseType]);
+ else
+ this.failure();
+ } else {
+ this.response = {text: this.xhr.responseText || '', xml: this.xhr.responseXML};
+ if (this.options.isSuccess.call(this, this.status))

@arian arian commented on an outdated diff Jan 18, 2013

Docs/Request/Request.md
console.log(parseInt(loaded / total * 100, 10));
}
});
myRequest.send();
+ var mySound = new Request({
+ method: 'get',
@arian

arian Jan 18, 2013

Owner

remove the extra spaces between : and 'get'.

Owner

arian commented Jan 18, 2013

I added some comments, sorry for the nitpicking, but if you fix that we can pull it 😄

leegee commented Jan 18, 2013

Fair points, thanks for making them.

On 18/01/2013 12:53, Arian Stolwijk wrote:

I added some comments, sorry for the nitpicking, but if you fix that
we can pull it 😄


Reply to this email directly or view it on GitHub
#2433 (comment).

@ibolmo ibolmo modified the milestone: 1.6, 1.5 Mar 3, 2014

ibolmo added the enhancement label Mar 3, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment