Opened 12 years ago

Closed 12 years ago

Last modified 11 years ago

#2924 closed enhancement (fixed)

Fix some of the fixmes in _base/xhr.js, and re-org to allow other io transports to use xhr infrastructure

Reported by: James Burke Owned by: alex
Priority: high Milestone:
Component: IO Version: 0.4.2
Keywords: Cc:
Blocked By: Blocking:

Description

Asking Alex to review:

The patch for this ticket fixes some of the fixmes in _base/xhr.js, in particular:

  • Preserves the user's input args
  • Better timeout support, with a "dojoType" property added to the Error object.
  • Make sure we xhr abort on an deferred error callback.

This patch also makes two methods publicly visible so they can be used by other io transports: dojo._ioWatch (was private _launch) and dojo._ioSetArgs (was part of private _setupForXhr).

Attachments (2)

2924.patch (10.7 KB) - added by James Burke 12 years ago.
Patch file #1
2924-2.patch (13.3 KB) - added by James Burke 12 years ago.
New version with load/error support, and puts form parsing inside dojo._ioSetArgs

Download all attachments as: .zip

Change History (11)

Changed 12 years ago by James Burke

Attachment: 2924.patch added

Patch file #1

comment:1 Changed 12 years ago by James Burke

File sizes of rhino-compressed dojo.js:

before patch: 64238
after patch:  64849

This patch adds 611 bytes.

comment:2 Changed 12 years ago by James Burke

Just added 2924-2.patch. This version has support for passing load and error callbacks as part of the args object passed to the public APIs. The load and error callbacks are added to the Deferred object underneath, and when they are called, they have their "this" set to the args object. They also receive the ioArgs as a second argument (the deferred result is the first).

This should make it very easy to make encapsulated IO calls that do not want to hold onto their args objects just so they can add callbacks to the Deferred object. Also, it allows easy access to the args state, since it is the "this" value in those callbacks.

File sizes of rhino-compressed dojo.js:

before patch:  64238
after patch2:  65099

Patch 2 adds 861 bytes compared to the unpatched version. It is 150 bytes larger than Patch 1.

I think that 150 bytes is worth avoiding having to explain Deferreds for the simple use cases (and not to mention the extra bytes the user will have to type to use the Deferreds directly).

This change would also work for the dojo.rpc case (the load and error callbacks are only added to the Deferred if they are functions).

Changed 12 years ago by James Burke

Attachment: 2924-2.patch added

New version with load/error support, and puts form parsing inside dojo._ioSetArgs

comment:3 Changed 12 years ago by James Burke

Just updated Patch 2 -- when trying to use the xhr infrasctructure for dojo.io.script (see ticket #2911), it seemed best to put the form parsing inside dojo._ioSetArgs().

File sizes of rhino-compressed dojo.js:

before patch:  64238
after patch2:  65016

Patch 2 adds 778 bytes compared to the unpatched version. It is 167 bytes larger than Patch 1. (Previous comment said the original Patch 2 was 150 bytes bigger than Patch 1, but that comment should have read "250 bytes bigger").

comment:4 Changed 12 years ago by alex

(In [8488]) merging the first part James' patch to add ScriptSrc? IO to Core. Refs #2924

comment:5 Changed 12 years ago by alex

Resolution: fixed
Status: newclosed

thanks for getting this revved and done. Marking fixed.

comment:6 Changed 12 years ago by alex

(In [8493]) fixes the tests. Refs #2924

comment:7 Changed 12 years ago by alex

(In [8540]) fixing XHR Post regression added in last rev. Refs #2924

comment:8 Changed 12 years ago by (none)

Milestone: 0.9M2

Milestone 0.9M2 deleted

comment:9 Changed 11 years ago by Adam Peller

(In [16566]) Change console.debug call to console.error, refs #2924, fixes #8583. (Do we really need a console message here at all if the error can be detected programatically? Style change refs #7250

Note: See TracTickets for help on using tickets.